Different strategy for installing platform-specific binaries

[eduardoboucas]

Hi! 👋🏻

We're looking to integrate esbuild with the Netlify CLI and some users are reporting permission-related errors when installing the application, which trace back to #369.

I understand that this is a result of the UID/GID switching that npm does when running scripts, which is problematic when the package is installed as root.

Rather than manually fetching the appropriate binary as part of the postinstall script, could the esbuild package list the various platform-specific packages as optionalDependencies, and shift to npm itself the responsibility of figuring out which packages to installed based on the cpu and os properties? We're doing this for one of our packages that also includes a Go binary and it works well.

I'm happy to contribute with a pull request, but I wanted to check whether this is something you'd be interested in, or even if it's something you've already considered and discarded for a particular reason.

Thanks in advance!

[evanw]

could the esbuild package list the various platform-specific packages as optionalDependencies

I have considered implementing the install script this way but I didn't go down that route because if I recall correctly it makes the installation output really noisy with all of the errors about unmet optional dependencies, which is a poor user experience. I'm also not totally sure what the guarantees around optional dependencies are (since I need them to be required, not optional) and if there are any situations where that could backfire and leave you with a broken package. Especially with all of the various alternate package managers other than npm that people like to use. Having my own install script gives me more control over these things.

I'd really rather not change the whole approach of the install script at this point because it's undergone a lot of evolution and has accumulated a lot of special cases that solve real problems. It's relatively stable at this point and changing it would risk breaking people, and would likely be a ton of work for me since I'd expect dozens of support requests from the fallout. It's at least not something I should do without a breaking change in the version number.

Depending on what you're doing, one possibility could be for you to just make your own package with all of esbuild's platform-specific packages as optional dependencies as you suggested. That should let you do this without any changes to esbuild itself. Doing this would at least be a less disruptive way of trying out the approach than involving esbuild's whole user base in the experiment.

[eduardoboucas]

Thanks for the additional context, @evanw. I completely understand your position.

If we do decide to explore this experiment, we'll make sure to report back with our findings, so that you can decide whether this is something you want to revisit in the future.

Thanks for your work. 🙌🏻

[eduardoboucas]

Depending on what you're doing, one possibility could be for you to just make your own package with all of esbuild's platform-specific packages as optional dependencies as you suggested

After looking into this for a bit, I think the main challenge would be to ensure that our package would benefit from any updates made upstream. We love esbuild and are excited to get the upcoming updates, so we were hoping to avoid anything that feels like a fork. The esbuild package includes not only the script for fetching the binaries, but also the entire JavaScript API, which would have to live on our side.

Would you be open to having a separate npm module that published from this repository, alongside the others? It would essentially be a copy of https://github.com/evanw/esbuild/blob/master/npm/esbuild/package.json, but with the optionalDependencies block and without the postinstall script.

If done from this repo, we would ensure that this module would include the latest changes, and it should be just a matter of adding an additional item to the Makefile.

Thanks!

[evanw]

I think it's worth getting an experimental package up with an example of this technique to try out. That should make it possible to test it with various package managers, operating systems, and different setting configurations and see what breaks.

[osdevisnot]

I wanted to record my own experience with this approach.

I've authored sorvor with platform specific packages as optionalDependencies.

This setup is mostly working on Mac and Linux, except few instances:

• NPM V7 - skips optional dependency checks and install all binaries for all platforms - issue link
• NPM V6 - does not create bin links in node_modules/.bin only when installing with --save-dev (works without --save-dev)

I still did not test this with yarn berry or windows (I don't have windows machine unfortunately).

PS: yarn works correctly with various combinations of yarn versions and install flags. This was rather surprising as I expected npm to be more stable.

[evanw]

Thanks very much for the report. It's good to have this additional data.

NPM V7 - skips optional dependency checks and install all binaries for all platforms

This seems like a pretty significant shortcoming. It seems like this means the approach just won't work after all. Is that correct?

[eduardoboucas]

This seems like a pretty significant shortcoming. It seems like this means the approach just won't work after all. Is that correct?

It is indeed an issue with npm 7. I have filed an issue and there's already a pull request for it, so it should be fixed with the next release of the npm CLI.

Regardless, as per the release notes of npm 7, «scripts are always run with the effective uid and gid of the working directory owner», which means that the permissions errors I reported should disappear. We've had users confirming that the errors are gone when installing with npm 7.

[osdevisnot]

It seems like this means the approach just won't work after all. Is that correct?

This is too early to call. As @eduardoboucas mentioned, the linked issue with NPM V7 is marked as closed and the fix will be available in next release.

NPM V7 makes this even more better, because user's do not see optional dependency warnings as observed with NPM V6 client as confirmed here