-
-
Notifications
You must be signed in to change notification settings - Fork 302
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
Enforce matching engines on npm install #481
Conversation
Yes. While there's no difference now, the problem might be fixed at some point, so it doesn't hurt to include it.
I wouldn't be surprised if this was the case. |
Co-Authored-By: Itai Steinherz <[email protected]>
It's currently a noop for `npm ci`, presumably because of an implementation quirk. But it's what we want to do, so we'll need to look into getting npm fixed.
All review feedback has been addressed. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @sholladay! 🙂 |
Fixes #480
This PR enables the engine-strict configuration for
npm install
when np reinstalls the dependencies. This will raise an error if the project or its dependencies have anengines.node
in package.json that are incompatible with the current environment.A question: should this be applied to the
npm ci
case as well? I rannpm ci --engine-strict
in a package whoseengines.node
was incompatible with the current Node version and it did not complain, whereasnpm install --engine-strict
did complain in the same situation. However, I did not test what happens when dependencies have mismatched engines. I also did not test using the npm config environment variable, only the command line flag. Perhaps an implementation quirk leads tonpm ci
loading configuration differently.