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

Remove ESLint verification when opting-out #10499

Merged
merged 2 commits into from Jun 2, 2021

Conversation

mrmckeb
Copy link
Contributor

@mrmckeb mrmckeb commented Feb 4, 2021

This PR follows #10170 and ensures that we no longer "verify" eslint and babel-eslint versions if the user has opted out of the plugin completely.

Related comment: #10170 (comment)

'jest',
'webpack',
'webpack-dev-server',
];

if (isESLintPluginEnabled) {
depsToCheck = [...depsToCheck, 'babel-eslint', 'eslint'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might as well just push eslint and babel-eslint into the array and then you can avoid spreading and reassigning the previous value. That's how I would do it anyway, but I don't feel strongly about this.

Copy link
Contributor Author

@mrmckeb mrmckeb Feb 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't disagree, it's a stylistic thing. Most of my the projects I work on are written to avoid mutation... and I just do it by default now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was updated to use filter(Boolean) instead, as we do in other places.

@iansu iansu modified the milestones: 4.0.3, 4.0.4 Feb 22, 2021
@mrmckeb mrmckeb force-pushed the feature/update-eslint-verify-checks branch from 0b0265d to 96fa29f Compare May 30, 2021 09:44
@mrmckeb mrmckeb force-pushed the feature/update-eslint-verify-checks branch from 96fa29f to 2a7c931 Compare May 30, 2021 09:47
@mrmckeb
Copy link
Contributor Author

mrmckeb commented May 30, 2021

I've tested this again today by using ESLint 7.10.0.

When the flag is not set:

There might be a problem with the project dependency tree.
It is likely not a bug in Create React App, but something you need to fix locally.

The react-scripts package provided by Create React App requires a dependency:

  "eslint": "^7.11.0"

Don't try to install it manually: your package manager does it automatically.
However, a different version of eslint was detected higher up in the tree:

  /Users/mrmckeb/Development/temp/cra-test/node_modules/eslint (version: 7.10.0)

With the flag CRA starts with no errors as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants