-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat: drop Node 4, 6, 8, 10, 12, 14 & 16 support #141
base: main
Are you sure you want to change the base?
feat: drop Node 4, 6, 8, 10, 12, 14 & 16 support #141
Conversation
BREAKING CHANGE: Requires Node@^18.18.0 || ^20.9.0 || >=21.1.0
@ljharb Settings should be updated to make |
|
||
on: [pull_request, push] | ||
|
||
jobs: | ||
tests: | ||
uses: ljharb/actions/.github/workflows/node.yml@main | ||
with: | ||
range: '>= 10' | ||
range: '^18.18.0 || ^20.9.0 || >=21.1.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.
Node 22 is out, so maybe don't need to support Node 21, which is unstable?
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.
I've set node.engines
to the same as ESLint is doing, but I'm happy to update if @ljharb wants to
@MichaelDeBoey could you please update the node range to v22 so it's mergeable? |
@talentlessguy that’s not what’s holding up this PR. |
@ljharb what's the problem then? |
the PR is slightly premature. It won't be merged until all the breaking changes are ready, both here and in eslint-plugin-jsx-a11y. |
@ljharb This is the way you requested the PRs on the e18e Discord: add them in small PRs, so they can be reviewed easily |
@MichaelDeBoey yep, and this is perfect once i'm ready :-) you don't need to do anything else. |
so in the end what kind of breaking changes have to be done in jsx-ast-utils? I still don't get it |
@talentlessguy for one, I have to make a branch for the major bump, and re-base these PRs onto it, and then I can merge them into that branch. I'll also have to TS-type the whole package, and likely a few other things. But either way, it's not necessary for you to get it, you'll just have to be patient. |
As @ljharb mentioned in #114 (comment), he's open for a breaking change right now
BREAKING CHANGE: Requires Node@^18.18.0 || ^20.9.0 || >=21.1.0