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

Handle react version detect errors #2336

Merged
merged 2 commits into from Jun 28, 2019

Conversation

abhishekdev
Copy link
Contributor

@abhishekdev abhishekdev commented Jun 28, 2019

What does this fix

The fix to #2276 (commit golopot@e2e246a) caused a regression only for scenarios where react version cannot be detected. Once the first warning is logged, a second try always results in the raw error being thrown and failure of the lint process.

Warning: React version was set to "detect" in eslint-plugin-react settings, but the "react" package is not installed. Assuming latest React version for linting.
Error: Error while loading rule 'react/jsx-no-bind': Cannot find module 'react' from 'project-path'
Occurred while linting project-path/.eslintrc.js
    at Function.module.exports [as sync] (project-path/node_modules/resolve/lib/sync.js:71:15)
    at detectReactVersion (project-path/node_modules/eslint-plugin-react/lib/util/version.js:19:31)
    at getReactVersionFromContext (project-path/node_modules/eslint-plugin-react/lib/util/version.js:39:25)
    at Object.testReactVersion (project-path/node_modules/eslint-plugin-react/lib/util/version.js:107:35)
    at usedPropTypesInstructions (project-path/node_modules/eslint-plugin-react/lib/util/usedPropTypes.js:271:48)
    at Function.componentRule (project-path/node_modules/eslint-plugin-react/lib/util/Components.js:763:37)
    at createRuleListeners (project-path/node_modules/eslint/lib/linter/linter.js:697:21)
    at Object.keys.forEach.ruleId (project-path/node_modules/eslint/lib/linter/linter.js:865:31)
    at Array.forEach (<anonymous>)
    at runRules (project-path/node_modules/eslint/lib/linter/linter.js:810:34)
npm ERR! Test failed.  See above for more details.

Solution

The warnedForMissingVersion flag should only control the warning message and not the handling of the error (which was the previous behavior)

I have included a test that ensures only a single error is present in stack. The issue can be reproduced by going back to the test commit. Here is the output for the case when it fails (i.e. before this fix):

 2) Version
       "after each" hook for "warns only once for failure to detect react ":

      AssertionError [ERR_ASSERTION]: [ [ 'Warning: React version was set to "detect" in eslint-plugin-react settings, but the "react" package is not installed. As... deepEqual []
      + expected - actual

      -[
      -  [
      -    "Warning: React version was set to \"detect\" in eslint-plugin-react settings, but the \"react\" package is not installed. Assuming latest React version for linting."
      -  ]
      -]
      +[]
      
      at Context.afterEach (tests/util/version.js:26:12)

- Warn only once for failure to detect react version

Ref: jsx-eslint#2276
- Adjust the conditions to always assume a latest react version when e.code === 'MODULE_NOT_FOUND'
- The `warnedForMissingVersion` flag only controls the warning message and not the handling of the error

Ref: jsx-eslint#2276
@ljharb ljharb merged commit 0215c38 into jsx-eslint:master Jun 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants