-
-
Notifications
You must be signed in to change notification settings - Fork 635
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
[New] add eslint 9 support #1009
Conversation
This change adds support for eslint v9. All three example projects have been updated to use the latest eslint, and the root package's dev and peer deps have been updated. In order to make this work with both v9 and older versions, I had to update the parser options helper to adjust the config passed into the `RuleTester`.
package.json
Outdated
"jest": "^24.9.0", | ||
"jest": "^29.7.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 can't upgrade jest, because jest 24 is the last version that supports node 6.
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.
Hmm. Ok, but jest 24 isn't able to resolve node:fs/promises
which eslint uses (so we hit this when importing RuleTester
from eslint). I could map node:fs/promises
to something like fs-extra
using jest's moduleNameMapper
? So the project is never going higher than jest 24?
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.
at some point we'll have a breaking change that drops older eslints/nodes, but i'm more likely to bail on jest entirely and use something less frictiony regardless.
can you map node:fs/promises
to fs/promises
, instead? the node:
prefix is pretty useless.
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.
After a few iterations, I got it working. I ended up having to map several built in node modules, as well as a couple of esm entry points of eslint packages, I also had to add a polyfill for structuredClone
, which eslint uses as part of RuleTester
and was added in node 17.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
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.
Everything's green now, if you want to take another look when you have the time. It may be worth considering dynamically installing a newer jest for v9 and removing all of those workarounds (in the same way each eslint version is being installed), but I'll leave that to you.
3db1f8c
to
4f21b85
Compare
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
f99300c
to
1231f61
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1009 +/- ##
==========================================
- Coverage 99.03% 98.73% -0.30%
==========================================
Files 107 107
Lines 1660 1666 +6
Branches 588 591 +3
==========================================
+ Hits 1644 1645 +1
- Misses 16 21 +5 ☔ View full report in Codecov by Sentry. |
f8e70cd
to
903bca2
Compare
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.
Seems good!
903bca2
to
deac4fd
Compare
When is this going out? Currently just using it from the repository directly. |
@iamtomcat i'm not sure how you'd do that in a practical sense, since we have a build process. It's only been 2 days - I'd suggest not using it at all until it's published. |
Hey @ljharb I appreciate you getting the change out, I wasn't trying to rush you, just wanted to get a time frame was all. Thanks again. |
it's now released; v6.10.0. |
@ljharb I think something is missing, there's no |
oh thanks, i always forget github releases (which don't matter; only npm matters) |
Oh, you're right, I didn't check npm 🙈 sorry. |
This change adds support for eslint v9. All three example projects have been updated to use the latest eslint, and the root package's dev and peer deps have been updated.
In order to make this work with both v9 and older versions, I had to update the parser options helper to adjust the config passed into the
RuleTester
. It was also necessary update jest to a newer version, in order to resolve eslint's use ofnode:fs/promises
.Note: the removed test cases are exact duplicates of other tests. v9 fails tests when its determined to be an exact duplicate.
Closes: #978