Relax ESLint peerDependency constraint to allow v9 #16414
Conversation
liuxingbaoyu
commented
Apr 6, 2024
•
liuxingbaoyu
commented
| Q | A |
|---|---|
| Fixed Issues? | Fixes: #16220 |
| Patch: Bug Fix? | |
| Major: Breaking Change? | |
| Minor: New Feature? | |
| Tests Added + Pass? | Yes |
| Documentation PR Link | |
| Any Dependency Changes? | |
| License | MIT |
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56756 |
| const noInvalidThisRule = new eslint.Linter().getRules().get("no-invalid-this"); | ||
| const noInvalidThisRule = ( | ||
| parseInt(eslintVersion, 10) >= 9 | ||
| ? require("eslint/use-at-your-own-risk").builtinRules |
There was a problem hiding this comment.
There was a problem hiding this comment.
See also ESLint's migration guide: https://eslint.org/docs/latest/use/migrate-to-8.0.0#-the-lib-entrypoint-has-been-removed.
liuxingbaoyu
force-pushed
the
support-eslint-v9
branch
from
ea2e77c
to
c8139eb
Compare
liuxingbaoyu
force-pushed
the
support-eslint-v9
branch
from
c8139eb
to
062f9a6
Compare
|
We need it shipped 🚢 and we need it now 🙏 |
There was a problem hiding this comment.
Can you add an ESLint 8 CI job? It will be similar to this job:
babel/.github/workflows/ci.yml
Line 508 in a84ec28
| const rule = new eslint.Linter().getRules().get("semi"); | ||
| const rule = new eslint.Linter({ configType: "eslintrc" }) | ||
| .getRules() | ||
| .get("semi"); |
There was a problem hiding this comment.
Why don't we need eslint/use-at-your-own-risk here?
There was a problem hiding this comment.
Because I don't know if eslint/use-at-your-own-risk exists in the old version, and I am worried about causing regression. :)
There was a problem hiding this comment.
Sorry I am still confused. For other rules, we guard eslint/use-at-your-own-risk with a version check:
const rule = (
parseInt(eslintVersion, 10) >= 9
? require("eslint/use-at-your-own-risk").builtinRules
: new eslint.Linter().getRules()
).get("builtin-rule-name");But this rule "semi" is handled in another way:
const rule = new eslint.Linter({ configType: "eslintrc" })
.getRules()
.get("semi");Is there any difference between semi and other builtin rules?
There was a problem hiding this comment.
They should be the same, I think it was a mistake on my part, thanks for catching it!
I initially misunderstood the comment at #16414 (comment). 🤦♂️
There was a problem hiding this comment.
No problem. I am just curious if there are any caveats within the semi rule. The new changes look good to me.
|
Semver-minor (support a new thing!) or patch (update an internal dep, and relax the peerDep constraint since it also works with ESLint 9)? |
|
I tend to treat it as a patch, because it is simple and we have no other new features. The patch version is simpler and faster. :) |
| @@ -51,7 +51,7 @@ | |||
| }, | |||
| "peerDependencies": { | |||
| "@babel/eslint-parser": "^7.11.0", | |||
| "eslint": "^8.9.0" | |||
| "eslint": "^8.9.0 || ^9.0.0" | |||
There was a problem hiding this comment.
Can we drop ESLint 8 support in Babel 8? ESLint 9 should prevail at the time we release Babel 8.
There was a problem hiding this comment.
I saw some tweets about how a large part of ESLint's ecosystem is still not compatible with ESLint 9 so many projects cannot migrate, I'd wait a bit to see how the situation evolves.
|
I am good to semi-patch. We can formally announce the ESLint 9 support in the 7.25 release post. |
|
Actually, maybe can we first release this as a prerelease to make sure that it's we don't need to adapt anything else in our plugins and parser? |
|
I published |
|
When we may expect it to get released? 👀 |
|
The prerelease is broken because I didn't publish it correctly, but I will re-publish it today. |
|
Does anyone know what's the ETA for it to get shipped 📦 with next semver patch? 🙄 |
|
🙏 |
|
Yeah let's release this in the next patch -- it's not adding any user-facing feature. |
liuxingbaoyu
added
PR: Internal 🏠
nicolo-ribaudo
changed the title
