Replace binary for rebuild cases

[Apollon77]

When doing an "npm rebuild" (e.g. on nodejs major upgrades) then nothing happens right now because node-pre-gyp simply assumes that binary is already there, but do not "rebuild" it ... This fix makes sure that existing binary gets replaced on install or rebuilds

Thanks for contributing!

• Have you updated CHANGELOG.md?

[Apollon77]

@zbjornson DO you see any way to merge and release this?

[TA2k]

@zbjornson This issue is really urgent and is affecting all people when they update their node version

[Apollon77]

I also resolved the Changelog conflict

[zbjornson]

npm rebuild is specific to rebuilding dependencies from source. Instead you should use npm install, which re-runs the install script, which detects the new NODE_MODULE_VERSION and installs the correct binary. I just verified that this is working correctly by installing canvas with Node.js v16, downgrading to v14 and rerunning npm install. Forcing a new binary download each time would unnecessarily slow it down. Can you try that please?

[Apollon77]

The problem is that the execution of "npm install" the the module directory has other effects because the module dependencies will then be duplicated in the library package directory and this then duplicates npm packages in the npm tree.

In my eyes it is good that also the "npm rebuild" command is supported by your library and has effects, because this is the usual way: After a nodejs upgrade the user executes "npm rebuild" in the "root package directory" and all dependent libraries in the tree rebuild themself if required.
Currently if the user do that e.g. serialport rebuilds itself correctly but canvas does not, so the application might not run. An "npm install" innthe root package will also not rebuild canvas because it will detect canvas as unchanged version and so not "npm install" in the canvas module ... This means that the user need to manualy find the package dir out our a cryptic crash message of the application and manually fix it himself ... This is not very convenient ...

Or do I miss anything?

[zbjornson]

the module dependencies will then be duplicated in the library package directory and this then duplicates npm packages in the npm tree.

I'm talking about running npm install from your package root directory, not in your_package/node_modules/canvas. There should be no duplication as a result.

[Apollon77]

@zbjornson Yes this sometimes work, and yes it might be a bit special what we do here because user install npm packages as plugins into a bigger npm tree (but should alo affect homebridge and other projects that does it that way.

The pitfallis as told above: npm will start creating a copy of all npm modules in the package directory ignoring other locations, so this then works ... but as soon as our package where we use it is updated then npm will remove all of these copies together with our package and replace it with the new version ... and then he "finds" the "old" (still incompatible) canvas in the npm tree and beause version did not changed npm will not re-"install" this ... so after an update of our package as "one in a big npm tree" the user have the issue again.

Also if user would do an "npm prune" the same would happen ... npm detect duplicate packages in the tree and the "most up one" will win and other are removed ... boom ... incompatible again

And this is basically caused by the "npm install that generates a subtree" ... so yes this is a working workaround but a support for a "npm rebuild" on top level would fix all of that.
And as said: most packages do exactly support that ... I only found two packages in our ecosystem that don't ... canvas and node-bluetooth-hci-socket :-)

[TA2k]

@zbjornson Is this your final decision to not support rebuild from sub node_modules in favor for a faster installation time?

[zbjornson]

@LinusU @chearon what do you think? I'm not opposed to this change if people expect this to work. It's the first time it's come up in the 5.5 years we've had prebuilds though and these alternatives are already possible:

• Pass the flag manually: npm rebuild --update-binary
• Use npm install

---

@Apollon77

npm will start creating a copy of all npm modules in the package directory ignoring other locations
...
npm detect duplicate packages in the tree and the "most up one" will win and other are removed

I'm not sure I understand correctly, but especially if you're using lockfiles these behaviors sound like bugs in npm...

[Apollon77]

I'm not sure I understand correctly, but especially if you're using lockfiles these behaviors sound like bugs in npm...

In fact the lockfile only exists on the most top level and if you execute npm install in a sub module from my knowledge the "somewhere parent" package-lock is ignored. I would expect that this is more "defined behaviour" than a npm bug, but honestly no idea what nom dows how in some cases in general ;-)

[zbjornson]

Yes, but I'm talking about running npm install from your package root directory, not in your_package/node_modules/canvas (#1982 (comment)).

[Apollon77]

Yes but "npm install" from the root directory will not fix a "canvas sub depenedency" because it detects that the version is already correct and soo will not also execute a "Npm install" for canvas ... this is exactly the issue ... A "npm rebuild" on root level is indeed then having effects on the camvas sub dep, but because of the originalscript then having no rebuild effect because "binaries are there"

[zbjornson]

I don't think that's the case. Both npm install and npm rebuild run the preinstall, install and postinstall scripts as documented here and here. npm install correctly updated a second-order dependency on canvas when I tested it.

[Apollon77]

You areright when it comes to updating the library ... but rebuild is usually "same version" case ... Or do I get you wrong?

[chearon]

And as said: most packages do exactly support that ... I only found two packages in our ecosystem that don't ... canvas and node-bluetooth-hci-socket :-)

Which other packages support this? It would be helpful to see.

My instinct was against this too, but I read up on npm rebuild (which I have never used) and it does sound like given what it's supposed to do and given the life cycle scripts available, it makes more sense.

[Apollon77]

Which other packages support this? It would be helpful to see.

Honestly ... would need to see because it only seems a relevant issue im node-gyp-pre is used and then depending on the parameters :-)
Lets phrase it that way: We work on the smart home system ioBrober with over 460 plugins (aka dynamically added own npm projects with their own sub-deps) and we only found canvas and node-bluetooth-hci-socket to be problematic when the users do nodejs updates ... in node-bluetooth-hci-socket we proposed the same change and was accepted there.

Commonly used in the pluings is unix-dgram, authenticate-pam (because used in core system, but not using node-gyp-pre) and also serialport (older versions work fine, new is migrated to NAPI :-) ) ... so hard to "find" them :-)

[TA2k]

@chearon @zbjornson
Have you come to a decision in your team?

[Apollon77]

@chearon @zbjornson Hey, do you have any update on that?

[chearon]

I still can't find any repos that do what you're suggesting and it's not what node-pre-gyp recommends. It would be helpful if you could show us an example of a popular repo that is doing this. The big drawback to merging this is that there could be a lot of unnecessary downloads.

[Apollon77]

Ok, then lets's "simply" thy this mapbox/node-pre-gyp#646 ... maybe we get insights from the makers of node-pre-gyp.

[zbjornson]

Sorry for sort of blocking this and then taking forever to follow up. I just played more with the behavior of npm and yarn and see how the behavior is unintuitive and unique to modules with prebuilds. I'm good with landing this.

[mateomarconi]

Hi,

I switched to Node.js version 18, but the 'canvas.node' module is compiled for version 16. I attempted to rebuild it without success; the error persists.

Error: The module '/app/node_modules/canvas/build/Release/canvas.node' was compiled against a different Node.js version using NODE_MODULE_VERSION 108. This version of Node.js requires NODE_MODULE_VERSION 93. Please try re-compiling or re-installing the module (for instance, using npm rebuild or npm install).

I am using a dependency that relies on the 'node-canvas' dependency.

Regarding this thread, how can I solve the issue?

[Apollon77]

I would say simply delete /app/node_modules/canvas/build/* and do "npm rebuild" un the directory /app/node_modules/canvas/