-
-
Notifications
You must be signed in to change notification settings - Fork 929
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
Remove Node.js 14 support #6785
Remove Node.js 14 support #6785
Conversation
|
@MohitMaheshwari1711 Thanks for the pull request. But I think this PR is too much for the reason I commented on #6740 (comment). |
I think if it has to be changed it should based on the LTS. |
@@ -201,6 +201,6 @@ | |||
"typescript": "^5.0.4" | |||
}, | |||
"engines": { | |||
"node": "^14.13.1 || >=16.0.0" | |||
"node": "^16.14.0 || >=18.0.0" |
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.
[suggest] I agree with @Mouvedia's suggestion on #6785 (comment). Node.js 16.13.0 is the first version of Node.js 16.x LTS series.
See https://nodejs.org/en/blog/release/v16.13.0
"node": "^16.14.0 || >=18.0.0" | |
"node": "^16.13.0 || >=18.0.0" |
The ls-engines
check passes:
$ npx ls-engines@latest
`node_modules` found; loading tree from disk...
┌────────┬─────────────────────────────────────────────────────────────────┐
│ engine │ Currently available latest release of each valid major version: │
├────────┼─────────────────────────────────────────────────────────────────┤
│ node │ v20.3.1, v19.9.0, v18.16.1, v17.9.1, v16.20.1 │
└────────┴─────────────────────────────────────────────────────────────────┘
┌───────────────────────────────────┬───────────────────────────┐
│ package engines: │ dependency graph engines: │
├───────────────────────────────────┼───────────────────────────┤
│ "engines": { │ "engines": { │
│ "node": "^16.13.0 || >= 18.0.0" │ "node": ">= 16.13" │
│ } │ } │
└───────────────────────────────────┴───────────────────────────┘
Your “engines” field allows fewer node versions than your dependency graph does.
...
Surely, dev dependencies require higher Node.js versions. But we can ignore them because they're just for dev, and we should respect engines.node
to support more users.
$ npx ls-engines@latest --dev
...
Your “engines” field does not exactly match your dependency graph‘s requirements!
...
I've changed the base branch from |
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.
@MohitMaheshwari1711 Could you consider addressing my review, please?
See #7020 |
Closes #6740
The issue mentioned that the minimum version required should be v16.0.0; as per my observations, some packages, such as jest and others need version ^16.14.0. I have attached a screenshot for further clarification. Hence I have added support for v16.14.0 rather than 16.0.0.