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.
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
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).
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
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!
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:
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.
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-androidalways fully rebuilds native modules, even if ran in succession with no relevant changes.Expected behavior
running
react-native run-androidis smart-ish about when to fully rebuild native modules (such as changes to the target asset'snode_modulesdirectory) and won't do unnecessary native builds on subsequent calls with no changes in betweenRepro steps
For context, let's use
better-sqlite3as 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
DeleteIncorrectPrebuildstask can inadvertently cause theBuildNpmModulestask to become an outdated task, causing the latter to run unnecessarily.Starting from scratch, we create our
nodejs-assetsfolder that nmrn will use. It looks like this:Note that we have set
BUILD_NATIVE_MODULES.txtto1to always build native modules if necessaryWe build for Android by running
react-native run-android(omitting irrelevant flags for brevity). Let's pretend that we're only building forarm64-v8a(although nmrn will built for all supported architectures in reality).Following the tasks defined by nmrn's build.gradle, observe that
DeleteIncorrectPrebuildswill not run - great! keep this in mind because it'll come back to haunt us in a later stepBuildNpmModulesruns and does all the fun node-gyp native build stuff. We end up with a new directory calledbuild/for each relevant native module found inAndroid/build/nodejs-native-assets-temp-build/nodejs-native-assets-arm64-v8a/nodejs-project/node_modules/. For example, inbetter-sqlite3we'll now have abuild/Release/directory that contains the generated.nodefile:Cool beans!
CopyBuiltNpmAssetswill move any directory hierarchy containing the.nodefiles fromAndroid/build/nodejs-native-assets-temp-build/nodjes-native-assets-arm64-v8a/nodejs-project/toAndroid/build/nodejs-native-assets/nodejs-native-assets-arm64-v8a/nodejs-modules/. We end up with something like this: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 thatBuildNpmModulesis 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
DeleteIncorrectPrebuildstask, we'll notice that what it's doing is deleting any.nodefiles that live in thenode_modulesof the relevant temp build directory, except for ones that live in aprebuilds/directory. So in our example, it's looking atAndroid/build/nodejs-native-assets-temp-build/nodjes-native-assets-arm64-v8a/nodejs-project/node_modules. Since step 4 produced.nodeartifacts and step 5 copied (note, not moved) them,DeleteIncorrectPrebuildsdetects files that it wants to delete and does so.Why is this important? It's important because the
BuildNpmModulestask defines the temp directory as a task output. SoDeleteIncorrectPrebuildsruns and does the files deletion in that directory, which causesBuildNpmModulesto 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:
build/directory that's created when building the native modules. I think that directory is somewhat standard for projects usingnode-gypbut could be mistaken. Updating the tasks implementation to account for this properly could solve this issue.Env info
nodejs-mobile-react-nativeversion: 16.17.10