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.

Sign up for GitHub

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

Jump to bottom

[React 19] eslint-plugin-react-compiler throws SyntaxErrors when it encounters .json or .graphql files #29107

Closed
trello-drew opened this issue May 16, 2024 · 3 comments
Closed

[React 19] eslint-plugin-react-compiler throws SyntaxErrors when it encounters .json or .graphql files #29107

trello-drew opened this issue May 16, 2024 · 3 comments
Labels
Component: Optimizing Compiler

Comments

@trello-drew
Copy link

trello-drew commented May 16, 2024 •
edited
Loading

Summary

If your linting command encompasses .json or .graphql files (e.g. eslint . package.json --ext .js,.jsx,.ts,.tsx,.graphql), and you have react-compiler/react-compiler enabled in your rules object:

  rules: {
    'react-compiler/react-compiler': 'error',

Then HermesParser will throw an exception and stop linting:

SyntaxError: Error while loading rule 'react-compiler/react-compiler': ';' expected (1:6)
query MemberProfile($id: ID!) {
      ^

This is the stack trace for the error:

Occurred while linting /<obfuscated>/app/scripts/controller/MemberProfileQuery.graphql
    at Object.parse (/<obfuscated>/web/node_modules/hermes-parser/dist/HermesParser.js:91:29)
    at Object.parse (/<obfuscated>/web/node_modules/hermes-parser/dist/index.js:129:28)
    at Object.create (/<obfuscated>/web/node_modules/eslint-plugin-react-compiler/dist/index.js:69121:42)
    at createRuleListeners (/<obfuscated>/web/node_modules/eslint/lib/linter/linter.js:895:21)
    at /<obfuscated>/web/node_modules/eslint/lib/linter/linter.js:1066:110
    at Array.forEach (<anonymous>)
    at runRules (/<obfuscated>/web/node_modules/eslint/lib/linter/linter.js:1003:34)
    at Linter._verifyWithoutProcessors (/<obfuscated>/web/node_modules/eslint/lib/linter/linter.js:1355:31)
    at Linter._verifyWithConfigArray (/<obfuscated>/web/node_modules/eslint/lib/linter/linter.js:1807:21)
    at Linter.verify (/<obfuscated>/web/node_modules/eslint/lib/linter/linter.js:1437:65)
    at Linter.verifyAndFix (/<obfuscated>/web/node_modules/eslint/lib/linter/linter.js:2068:29)
    at verifyText (/<obfuscated>/web/node_modules/eslint/lib/cli-engine/cli-engine.js:254:48)
    at CLIEngine.executeOnFiles (/<obfuscated>/web/node_modules/eslint/lib/cli-engine/cli-engine.js:834:28)
    at ESLint.lintFiles (/<obfuscated>/web/node_modules/eslint/lib/eslint/eslint.js:551:23)
    at Object.execute (/<obfuscated>/web/node_modules/eslint/lib/cli.js:421:36)
    at async main (/<obfuscated>/web/node_modules/eslint/bin/eslint.js:152:22)

The workaround to solve this right now is to disable the rule for these files using an override:

  overrides: [
    {
      files: ['*.json'],
      rules: {
        'react-compiler/react-compiler': 'off',
      },
    },
    {
      files: ['*.graphql'],
      rules: {
        'react-compiler/react-compiler': 'off',
      }
    }
  ]
@josephsavona
Copy link
Contributor

Thanks for filing this!

@chentsulin
Copy link
Contributor

Found another tricky issue, hermes parser doesn't support top-level await:
facebook/hermes#1352

@josephsavona
Copy link
Contributor

I believe that @poteto already fixed this on the latest release — can you confirm?

@poteto poteto mentioned this issue May 29, 2024
Merged
poteto added a commit that referenced this issue May 29, 2024
@poteto
[compiler:eslint] Don't crash if hermes parser fails to parse
ed95909 
Eslint rules should never throw, so if we fail to parse with Babel or
Hermes, we should just ignore the error. This should fix issues such as
trying to run the eslint rule on non tsx|ts|jsx|js files, Hermes parser
not supporting certain JS syntax, etc.

Fixes #29107

ghstack-source-id: 6d114ebdbee0161d3747634aa956da3d71de39ed
Pull Request resolved: #29631
poteto added a commit that referenced this issue May 29, 2024
@poteto
[compiler:eslint] Don't crash if hermes parser fails to parse
777d7ea 
Eslint rules should never throw, so if we fail to parse with Babel or
Hermes, we should just ignore the error. This should fix issues such as
trying to run the eslint rule on non tsx|ts|jsx|js files, Hermes parser
not supporting certain JS syntax, etc.

I didn't add a test for this as our eslint-rule-tester config uses
hermes-eslint parser, so it wasn't possible to add a top level await as
it would crash hermes-eslint before our rule was triggered. Similarly I
couldn't add a test for non-JS files as it would not be parseable by
hermes-eslint.

Fixes #29107

ghstack-source-id: 6d114ebdbee0161d3747634aa956da3d71de39ed
Pull Request resolved: #29631
poteto added a commit that referenced this issue May 31, 2024
@poteto
[compiler:eslint] Don't crash if hermes parser fails to parse
a943066 
Eslint rules should never throw, so if we fail to parse with Babel or
Hermes, we should just ignore the error. This should fix issues such as
trying to run the eslint rule on non tsx|ts|jsx|js files, Hermes parser
not supporting certain JS syntax, etc.

I didn't add a test for this as our eslint-rule-tester config uses
hermes-eslint parser, so it wasn't possible to add a top level await as
it would crash hermes-eslint before our rule was triggered. Similarly I
couldn't add a test for non-JS files as it would not be parseable by
hermes-eslint.

Fixes #29107

ghstack-source-id: 60afcdb89ab4a8d2e4697cc50c5490803e7cbeac
Pull Request resolved: #29631
@poteto poteto closed this as completed in 113c8e7 May 31, 2024
github-actions bot pushed a commit that referenced this issue May 31, 2024
@poteto
[compiler:eslint] Don't crash if hermes parser fails to parse
fc83ec7 
Eslint rules should never throw, so if we fail to parse with Babel or
Hermes, we should just ignore the error. This should fix issues such as
trying to run the eslint rule on non tsx|ts|jsx|js files, Hermes parser
not supporting certain JS syntax, etc.

I didn't add a test for this as our eslint-rule-tester config uses
hermes-eslint parser, so it wasn't possible to add a top level await as
it would crash hermes-eslint before our rule was triggered. Similarly I
couldn't add a test for non-JS files as it would not be parseable by
hermes-eslint.

Fixes #29107

ghstack-source-id: 60afcdb89ab4a8d2e4697cc50c5490803e7cbeac
Pull Request resolved: #29631

DiffTrain build for commit 113c8e7.
github-actions bot pushed a commit that referenced this issue May 31, 2024
@poteto
[compiler:eslint] Don't crash if hermes parser fails to parse
23fac22 
Eslint rules should never throw, so if we fail to parse with Babel or
Hermes, we should just ignore the error. This should fix issues such as
trying to run the eslint rule on non tsx|ts|jsx|js files, Hermes parser
not supporting certain JS syntax, etc.

I didn't add a test for this as our eslint-rule-tester config uses
hermes-eslint parser, so it wasn't possible to add a top level await as
it would crash hermes-eslint before our rule was triggered. Similarly I
couldn't add a test for non-JS files as it would not be parseable by
hermes-eslint.

Fixes #29107

ghstack-source-id: 60afcdb89ab4a8d2e4697cc50c5490803e7cbeac
Pull Request resolved: #29631

DiffTrain build for [113c8e7](113c8e7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: Optimizing Compiler
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@chentsulin @josephsavona @eps1lon @trello-drew