-
Notifications
You must be signed in to change notification settings - Fork 46
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: support eslint v8 #123
Conversation
8c11648
to
5202a5d
Compare
Hmm, one intermediary step could be to migrate to EDIT: See #128 |
1880ecb
to
3192cf9
Compare
I've confirmed that commenting out the |
8a52924
to
c4b4ccb
Compare
🎉 |
I’d strongly suggest keeping this PR in draft until eslint v8 is fully released, to avoid needing to support prereleases. |
Yup, agreed 🙂, just getting #128 is fine for me since it'll allow consumers to use v8 (albeit with peer dep warnings) instead of blocking them |
Version 8 has been released, but I don't wanna merge this with |
I’ll try to release that today or tomorrow. |
This reverts commit 9111113.
🥳 |
2 main breaking changes affect us (beyond node version requirement)
CLIEngine
The first one I believe to be no issue since we can check which is available and adjust our API usage accordingly (and it being async doesn't matter).
However, the second one stumped me a bit. It might be enough to - depending on the eslint version - use
overrideConfig
instead? This is the error:Note that I don't think we should necessarily merge this until ESLint v8 is released as stable, but it would be good to be ready to not block others.
Lastly,
eslint-plugin-import
explodes horribly due to using deep imports (import-js/eslint-plugin-import#2211), so we should skip our lint run somehow, at least until that plugin adds support.EDIT: Actually, looks like import-js/eslint-plugin-import#2194 should do it once it's released