[android] Native modules are always rebuilt despite lack of relevant changes

[achou11]

Attempting to upgrade from v0.8.1 to v16.17.10 and running into a regression where native npm modules are always rebuilt by nmrn (nodejs-mobile-react-native, abbreviated) despite changes not occurring for the relevant modules.

Actual behavior

running react-native run-android always fully rebuilds native modules, even if ran in succession with no relevant changes.

Expected behavior

running react-native run-android is smart-ish about when to fully rebuild native modules (such as changes to the target asset's node_modules directory) and won't do unnecessary native builds on subsequent calls with no changes in between

Repro steps

For context, let's use better-sqlite3 as the native module in question for our application. We DO NOT use https://github.com/staltz/prebuild-for-nodejs-mobile right now (although maybe we should) to create native prebuilds for them before running the build process for our app.

tl;dr is that the DeleteIncorrectPrebuilds task can inadvertently cause the BuildNpmModules task to become an outdated task, causing the latter to run unnecessarily.

1. Starting from scratch, we create our nodejs-assets folder that nmrn will use. It looks like this:

   

   Note that we have set BUILD_NATIVE_MODULES.txt to 1 to always build native modules if necessary

2. We build for Android by running react-native run-android (omitting irrelevant flags for brevity). Let's pretend that we're only building for arm64-v8a (although nmrn will built for all supported architectures in reality).

3. Following the tasks defined by nmrn's build.gradle, observe that DeleteIncorrectPrebuilds will not run - great! keep this in mind because it'll come back to haunt us in a later step

4. BuildNpmModules runs and does all the fun node-gyp native build stuff. We end up with a new directory called build/ for each relevant native module found in Android/build/nodejs-native-assets-temp-build/nodejs-native-assets-arm64-v8a/nodejs-project/node_modules/. For example, in better-sqlite3 we'll now have a build/Release/ directory that contains the generated .node file:

   

   Cool beans!

5. CopyBuiltNpmAssets will move any directory hierarchy containing the .node files from Android/build/nodejs-native-assets-temp-build/nodjes-native-assets-arm64-v8a/nodejs-project/ to Android/build/nodejs-native-assets/nodejs-native-assets-arm64-v8a/nodejs-modules/. We end up with something like this:

   

6. Now, try running step 2 again i.e. run react-native run-android. You'll notice that step 4 reruns again and therefore we rebuild the native modules from scratch, even if nothing has changed! The expectation here is that BuildNpmModules is marked as an up-to-date task and doesn't run.

---

What's the problem?

Let's revisit the note from step 3...

If we look at the implementation of the DeleteIncorrectPrebuilds task, we'll notice that what it's doing is deleting any .node files that live in the node_modules of the relevant temp build directory, except for ones that live in a prebuilds/ directory. So in our example, it's looking at Android/build/nodejs-native-assets-temp-build/nodjes-native-assets-arm64-v8a/nodejs-project/node_modules. Since step 4 produced .node artifacts and step 5 copied (note, not moved) them, DeleteIncorrectPrebuilds detects files that it wants to delete and does so.

Why is this important? It's important because the BuildNpmModules task defines the temp directory as a task output. So DeleteIncorrectPrebuilds runs and does the files deletion in that directory, which causes BuildNpmModules to be an "outdated" task that runs again. This becomes cyclical as a result if nothing is changed from the steps above.

Solution

Not sure what the proper solution is, but a couple of initial thoughts:

• in step 5, if the directories were moved instead of copied, that would help with this issue. Not sure if that's ideal for debugging reasons though
• these tasks don't account for the generated build/ directory that's created when building the native modules. I think that directory is somewhat standard for projects using node-gyp but could be mistaken. Updating the tasks implementation to account for this properly could solve this issue.

---

Env info

• OS: macOS 11.7.6 (Big Sur)
• React Native version: 0.66.5
• Gradle version: 6.9
• nodejs-mobile-react-native version: 16.17.10
• node version: 16.17.1
• npm version: 6.14.18