-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Breaking: drop node v10/v13/v15 (fixes #14023) #14592
Conversation
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 whenever we start merging breaking changes for v8.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.
We should also update prerequisites in README and getting-started.
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!
Note to whomever merges this: try reverting #14771, which pinned |
#14771 has been merged. so I can revert it after rebasing. |
Re |
Co-authored-by: Brandon Mills <btmills@users.noreply.github.com>
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
f35c05a
to
219ad2d
Compare
rebased upstream master. |
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.
verify_files doesn't specify Node version, and the current default seems to be v10.24.1
. After merging this PR, that job will be running on an unsupported version.
https://github.com/eslint/eslint/pull/14592/checks?check_run_id=3067449292 was successful with warnings, but the check will eventually break when we add more code that doesn't work in Node 10.
npm WARN notsup Unsupported engine for eslint@7.30.0: wanted: {"node":"^12.22.0 || ^14.17.0 || >=16.0.0"} (current: {"node":"10.24.1","npm":"6.14.12"})
npm WARN notsup Not compatible with your version of node/npm: eslint@7.30.0
npm WARN notsup Unsupported engine for fs-extra@10.0.0: wanted: {"node":">=12"} (current: {"node":"10.24.1","npm":"6.14.12"})
npm WARN notsup Not compatible with your version of node/npm: fs-extra@10.0.0
I think we should specify the Node version, but not sure which one would make the most sense - 12.22.0, 12.x, 14.x, or 16.x?
Seems like 14.x would be the safest bet? Guaranteed ESM and stable. |
Just wondering if there was a particular reason for pinning to 4.17.0 rather than an earlier version of 14? Our project is an installed application, rather than a library & is currently pinned to 14.16.1. For us bumping Node to be able to upgrade a dev dependency doesn't make sense as it would impact 1000s of people 😬 Is there any chance you'd accept a PR to drop the pinned version by 1 version to 14.16.1? |
refs eslint#14592 (comment) - this will allow the Ghost project to update their version of EsLint without impacting on all of our users
it was discussed in #14023 (comment). a workaround is to use |
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:
What changes did you make? (Give an overview)
fixes #14023
Is there anything you'd like reviewers to focus on?
no.