Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow an env var to skip install.js for this package only #1311

Closed
wants to merge 1 commit into from

Conversation

rubythulhu
Copy link

This will allow us (hopefully; I have no confirmation that skipping this step results in a successful build which doesn't fail at runtime -- see #1310) to disable this install script for this package only, something that none of the major node package managers allow (couldn't find a way in any of npm/yarn/pnpm to do this for only this package -- only globally disable scripts, when find node_modules -name package.json | wc -l is almost 7000 files, i'm not sure how i'm ever supposed to validate that "globally disallow lifecycle scripts" doesn't break anything).

In any case, since this package claims to be "pure javascript", it should have an easy and explicit way to disable C++ modules, and actually be "pure" javascript.

But, again, I don't know if this even makes a difference. Our CI "forgot" to bundle binary modules, and this caused our production services to fail at run-time because it couldn't load this module. I'm fairly sure there is something else I need to do in this PR to disable this binary module, but I don't know what it is. To me, that means that if this script is disabled, this binary module won't appear in our codebase, and we'd get the same runtime exception thrown about not being able to load it as we did when our CI skipped adding it (because "don't copy it into the codebase" and "never generate it in the first place" SHOULD behave the same when "it doesn't exist", which is either "this is ok, i can live w/o this module" or "i fail at runtime b/c this module isn't there". For us, it is always the latter)

@mscdex
Copy link
Owner

mscdex commented Jun 17, 2023

couldn't find a way in any of npm/yarn/pnpm to do this for only this package -- only globally disable scripts

Sounds like that's a feature request you should create for whichever package manager. Better to have such features live where they ought to instead of every package that has optional addons use their own non-standard thing.

@rubythulhu
Copy link
Author

closing this pr for reasons outlined in #1310. this change wouldn't even help me fix my issue.

@rubythulhu rubythulhu closed this Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants