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

Set Node/npm Engines #2001

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Set Node/npm Engines #2001

merged 1 commit into from
Jun 29, 2023

Conversation

puntope
Copy link
Contributor

@puntope puntope commented Jun 26, 2023

Changes proposed in this Pull Request:

  • Set the strict-engines directive in .npmrc
  • Set the engines in package.json

Detailed test instructions:

  1. Run nvm use 14 and see the npm compilation builds
  2. Run nvm use 18 (or any other version, not 14) and see the npm compilation fails

Additional details:

Changelog entry

Dev - Set engines for the repository

@puntope puntope self-assigned this Jun 26, 2023
@github-actions github-actions bot added the changelog: dev Developer-facing only change. label Jun 26, 2023
@puntope puntope requested a review from a team June 26, 2023 14:36
@puntope puntope marked this pull request as ready for review June 26, 2023 14:44
Copy link
Member

@ianlin ianlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this, I tested with Node 14 and the npm compilation was ok, using Node 12 gave me the expected error below:

npm ERR! code ENOTSUP
npm ERR! notsup Unsupported engine for [email protected]: wanted: {"node":"^14","npm":"^6"} (current: {"node":"12.22.1","npm":"6.14.12"})
npm ERR! notsup Not compatible with your version of node/npm: [email protected]
npm ERR! notsup Not compatible with your version of node/npm: [email protected]
npm ERR! notsup Required: {"node":"^14","npm":"^6"}
npm ERR! notsup Actual:   {"npm":"6.14.12","node":"12.22.1"}

However when using Node 16 and 18, the error was not Unsupported engine, but this:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/prettier
npm ERR!   dev prettier@"npm:[email protected]" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer prettier@">=2" from @wordpress/[email protected]
npm ERR! node_modules/@wordpress/prettier-config
npm ERR!   dev @wordpress/prettier-config@"^1.1.2" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR!
npm ERR! For a full report see:
npm ERR! /Users/ianlin/.npm/_logs/2023-06-28T08_32_10_165Z-eresolve-report.txt

It's weird that the error is not Unsupported engine even though we added ^14 in package.json. What do you think about this behaviour?

@puntope
Copy link
Contributor Author

puntope commented Jun 28, 2023

It's weird that the error is not Unsupported engine even though we added ^14 in package.json. What do you think about this behaviour?

When I used Nonde 16 I got the right message.

npm ERR! code EBADENGINE
npm ERR! engine Unsupported engine
npm ERR! engine Not compatible with your version of node/npm: [email protected]
npm ERR! notsup Not compatible with your version of node/npm: [email protected]
npm ERR! notsup Required: {"node":"^14","npm":"^6"}
npm ERR! notsup Actual:   {"npm":"8.11.0","node":"v16.16.0"}

npm ERR! A complete log of this run can be found in:

However, if I delete package-lock.json yes I got the mentioned error. My understanding is that NPM tries first to resolve the dependency tree, and after, it checks the engines.

@puntope puntope requested a review from ianlin June 28, 2023 13:38
Copy link
Member

@ianlin ianlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification, I tried not deleting package-lock.json and it showed the expected error message.

@puntope puntope merged commit e1ec494 into develop Jun 29, 2023
@puntope puntope deleted the dev/set-engines branch June 29, 2023 09:52
@eason9487 eason9487 mentioned this pull request Jul 11, 2023
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: dev Developer-facing only change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants