Skip to content
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

[eslint-patch] Upgrade to ESLint 9 #4635

Closed
Nixie24 opened this issue Apr 11, 2024 · 14 comments · Fixed by #4719
Closed

[eslint-patch] Upgrade to ESLint 9 #4635

Nixie24 opened this issue Apr 11, 2024 · 14 comments · Fixed by #4719
Labels
effort: medium Needs a somewhat experienced developer help wanted If you're looking to contribute, this issue is a good place to start!

Comments

@Nixie24
Copy link

Nixie24 commented Apr 11, 2024

ESLint v9.0.0 is released 🎉

It would be awesome to have official ESLint 9 support. 🙂

@iclanton
Copy link
Member

I'm pretty sure there are a bunch of breaking changes in ESLint 9, so this may take some effort. If someone wants to take that on, we'd happily take it. For now, it'll be a lower priority for our team.

@iclanton iclanton added help wanted If you're looking to contribute, this issue is a good place to start! effort: medium Needs a somewhat experienced developer labels Apr 15, 2024
@iclanton iclanton moved this from Needs triage to Low priority in Bug Triage Apr 15, 2024
@kirill-konshin
Copy link

Even with recent fixup https://eslint.org/blog/2024/05/eslint-compatibility-utilities/ it fails:

Oops! Something went wrong! :(

ESLint: 9.2.0

Error: Failed to patch ESLint because the calling module was not recognized.
If you are using a newer ESLint version that may be unsupported, please create a GitHub issue:
https://github.com/microsoft/rushstack/issues
    at Object.<anonymous> (/xxx/node_modules/@rushstack/eslint-patch/lib/_patch-base.js:165:19)
    at Module._compile (node:internal/modules/cjs/loader:1375:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1434:10)
    at Module.load (node:internal/modules/cjs/loader:1206:32)
    at Module._load (node:internal/modules/cjs/loader:1022:12)
    at Module.require (node:internal/modules/cjs/loader:1234:19)
    at require (node:internal/modules/helpers:176:18)
    at Object.<anonymous> (/xxx/node_modules/@rushstack/eslint-patch/lib/modern-module-resolution.js:11:23)
    at Module._compile (node:internal/modules/cjs/loader:1375:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1434:10)

Just in case this helps...

The fixup works with configs (actual objects returned from config), and eslint-patch is usually called inside config. It might be useful to just disable some functionality if ESLint 9 is detected...

@kirill-konshin
Copy link

Thanks @nzakas for prompt response and fix. I assume we now have to wait for release and propagation to other packages like Next.js rules set?

@D4N14L
Copy link
Member

D4N14L commented May 16, 2024

@kirill-konshin yes. Publish is manually triggered. You can either choose to wait for the next publish, or you can request a publish from one of the senior maintainers.

@DmitryMarkov
Copy link

Even with recent fixup https://eslint.org/blog/2024/05/eslint-compatibility-utilities/ it fails:

Same for me

@kirill-konshin
Copy link

@D4N14L was it published? I'm getting the same error #4965 (comment) but with shareable config.

@D4N14L
Copy link
Member

D4N14L commented Oct 30, 2024

@kirill-konshin yes it is published, though I don't know if support for shareable configs requires any additional work. Could you share a repro?

@kirill-konshin
Copy link

kirill-konshin commented Oct 30, 2024

@D4N14L sure, here you go: https://codesandbox.io/p/devbox/nqs2yk?file=%2Feslint.config.mjs

Just run yarn lint.

The problem is interference with newer Yarn it seems. Also I purposely made it a monorepo.

Here's with the Yarn 1: https://stackblitz.com/edit/stackblitz-starters-zxpfgk?file=eslint.config.mjs,package.json

It does not reproduce there directly, but if you download a zip and run

$ yarn set version berry
$ yarn
$ yarn lint

Then it will fail.

@kirill-konshin
Copy link

@D4N14L any chances to have this fixed? Can I help?

@D4N14L
Copy link
Member

D4N14L commented Nov 6, 2024

@kirill-konshin to be honest I don't think I'll be able to look any time soon. If you can create a fix or investigate a fix, that would be appreciated.

@kirill-konshin
Copy link

@D4N14L I don't really think I can, since the error originates from the module resolution patch and Yarn's nodeLinker: node-modules config.

BUT! On a positive side, I've just tested my custom config with Yarn's nodeLinker: pnpm (not pnp!) and it magically worked just fine.

So in addition to my reproduction scenario before, I can reliably say it works well with Yarn 1 and Yarn 2+ with nodeLinker: node-modules.

@D4N14L
Copy link
Member

D4N14L commented Nov 19, 2024

@kirill-konshin nice! So then seems possibly an issue with pnp compatibility in ESLint instead of anything that we have done. Glad to hear :)

@kirill-konshin
Copy link

@kirill-konshin nice! So then seems possibly an issue with pnp compatibility in ESLint instead of anything that we have done. Glad to hear :)

Not PNP but original node-modules linker...

@D4N14L
Copy link
Member

D4N14L commented Nov 20, 2024

Ah interesting. Then possibly an issue with shadow dependencies given that PNPM brings more dependency correctness with its algorithm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: medium Needs a somewhat experienced developer help wanted If you're looking to contribute, this issue is a good place to start!
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants