search

atom / node-keytar

Native Password Node Module
https://atom.github.io/node-keytar
MIT License
1.37k stars 193 forks source link

Switch to prebuildify & node-gyp-build #462

Open TylerLeonhardt opened 2 years ago

TylerLeonhardt commented 2 years ago

Summary

prebuild-install is recommending:

Instead of prebuild paired with prebuild-install, we recommend prebuildify paired with node-gyp-build.

With prebuildify, all prebuilt binaries are shipped inside the package that is published to npm, which means there's no need for a separate download step like you find in prebuild. The irony of this approach is that it is faster to download all prebuilt binaries for every platform when they are bundled than it is to download a single prebuilt binary as an install script.

From https://github.com/prebuild/prebuild-install#note

Right now, prebuild-install installs 759KBs of node_modules none of which need to ship in our product because keytar is standalone AFAICT.

Motivation

We, in vscode, are trying to slim down the size of the package and because of prebuild-install we are introducing 759kbs of stuff that isn't used at all.

Describe alternatives you've considered

In vscode we have considered installing keytar to a separate location and then copying the files over to our build to slim down our package but it would be nice if keytar followed the recommendation here by prebuild which, in theory, will slim down the build.

Additional context

sergiou87 commented 2 years ago

Hey @TylerLeonhardt !

Thank you for your suggestion, I didn't know about the existence of prebuildify. Looking at the prebuild-install link you provided, it mentions this downside:

The installed npm package is larger on disk. Using Node-API alleviates this because Node-API binaries are runtime-agnostic and forward-compatible.

If I understand this correctly, you get rid of the 759KB from prebuild-install… but will get new binaries of keytar that you might or might not need. Looking at the latest release of keytar we're talking about ~800KB that outweigh the 759KB from prebuild-install.

Is that correct? Or will your bundler get rid of the binaries that aren't needed? 🤔

In case my reasoning is wrong and this change is actually worth doing, we can't promise any ETA 😢 Right now keytar is being maintained by the GitHub Desktop team, which is quite small. We definitely accept contributions to the repository, but one idea I discussed with one of your teammates a few months ago was transferring ownership of keytar to the VS Code team, since it was kind of a core dependency to you.

thegecko commented 2 years ago

Also see benefits as outlined by @vweevers in https://github.com/atom/node-keytar/issues/255#issuecomment-598727183