-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add ESLint 8 support to @babel/eslint-parser
#13782
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 8d1e62e:
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/49490/ |
53e4b51
to
4a07c07
Compare
6790469
to
0b73f6b
Compare
q.start -= 1; | ||
if (q.tail) { | ||
q.end += 1; | ||
} else { | ||
q.end += 2; |
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.
Does this close #13060?
|
||
const staticKw = { type: "Keyword", value: "static" }; | ||
|
||
expect(babylonAST.tokens[3]).toMatchObject(staticKw); |
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 avoid hard-coded tokens index by a class wrapper
function getFirstToken(source) {
return parseForESLint(`class A { ${source} }`).ast.tokens[3];
}
expect(getFirstToken("static m() {}").toMatchObject(staticKw)
// ...
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.
I'd prefer to do that in a separate PR, since we would need to update the whole file.
I don't think we need docs for this, since we don't mention anywhere that we support ESLint 7. |
55df468
to
7168c3d
Compare
I cannot reproduce the failures locally, and I'm confused because Node.js 17 (latest) fails but Node.js 16 doesn't 🤔 |
@@ -111,6 +114,7 @@ module.exports = function (api) { | |||
case "development": |
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.
I think we should also add dynamicESLintVersionCheck = true;
fore "test"
, the CI begins failing when I remove the plugin for test
: JLHwung@f13dad3
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.
Thanks!
The parser can check the ESLint version using
require("eslint/package.json")
(since it's a peer dependency). However, we need an escape hatch to switch version on-the-fly during our tests.I don't think we need any change to
@babel/eslint-plugin
: we could potentially make some rules no-op, but they are already effectively no-op with ESLint 8.