- 
                Notifications
    You must be signed in to change notification settings 
- Fork 48
feat: support eslint v8 #123
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
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)
CLIEngineThe 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
overrideConfiginstead? 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-importexplodes 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