The Wayback Machine - http://web.archive.org/web/20210212163520/https://github.com/facebook/create-react-app/issues/9887
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

After update to 4.0 lint errors are blocking the rendering #9887

Closed
Poyoman39 opened this issue Oct 24, 2020 · 55 comments
Closed

After update to 4.0 lint errors are blocking the rendering #9887

Poyoman39 opened this issue Oct 24, 2020 · 55 comments

Comments

@Poyoman39
Copy link

@Poyoman39 Poyoman39 commented Oct 24, 2020

Describe the bug

With CRA 3.x lint errors where reported as warning in the console. Now it is blocking the rendering :
image

Expected behaviour

I would like to have by default the old behaviour (warning in console), or at least a way to switch back to this old behaviour


Edit:

@unframework give bellow a nice analysis about this issue: #9887 (comment)_

Solutions & workarounds

  • Implement a way to disable eslint in CRA builds/dev (env variables ...)
  • Use two eslint config files. One for text editor, and one for React.
  • Clean your code or change severity of eslint rules in your eslintrc file. (change them to warning for instance)
@chickenwing
Copy link

@chickenwing chickenwing commented Oct 24, 2020

I always get these compile fails on eslint errors also in CRA 3.x (which is fine).

@mstykow
Copy link

@mstykow mstykow commented Oct 24, 2020

It depends. For example a simple console.log now also prevents compiling. This was not the case with 3.4.x. There is always the flag TSC_COMPILE_ON_ERROR=true you can set in your .env to compile anyways.

@fred172
Copy link

@fred172 fred172 commented Oct 26, 2020

This also breaks on prop warnings, which is very intrusive during development. It would be nice if this could be fixed...

@RosenTomov
Copy link

@RosenTomov RosenTomov commented Oct 26, 2020

Updated:

Add

emitWarning: true,
failOnWarning: false,

In config/webpack.config.js -> ESLintPlugin

      new ESLintPlugin({
        // Plugin options
        extensions: ['js', 'mjs', 'jsx', 'ts', 'tsx'],
        formatter: require.resolve('react-dev-utils/eslintFormatter'),
        eslintPath: require.resolve('eslint'),
        context: paths.appSrc,
        emitWarning: true, // new
        failOnWarning: false, // new
        // ESLint class options
        cwd: paths.appPath,
        resolvePluginsRelativeTo: __dirname,
        baseConfig: {
          extends: [require.resolve('eslint-config-react-app/base')],
          rules: {
            ...(!hasJsxRuntime && {
              'react/react-in-jsx-scope': 'error',
            }),
          },
        },
      }),
@Saibamen
Copy link

@Saibamen Saibamen commented Oct 26, 2020

failOnError: false, Don't do that... Skip warnings only

@rus-yurchenko
Copy link

@rus-yurchenko rus-yurchenko commented Oct 26, 2020

Even more, the compilation time became much longer because it now extends the ESlint config and runs lint on every change, which eliminates the speed of fast refresh. I used ESlint only as a part of lint-staged before and want to continue to use it as an optional thing that won't slow down compilation time.

@RosenTomov
Copy link

@RosenTomov RosenTomov commented Oct 26, 2020

Try with caching, again add
cache: true,

in ESLintPlugin options.

@Saibamen
Copy link

@Saibamen Saibamen commented Oct 26, 2020

Caching fixed in this PR: #9911

@asdfasdfasdfasdfff
Copy link

@asdfasdfasdfasdfff asdfasdfasdfasdfff commented Oct 27, 2020

There is a temporary workaround, but it works for me for now. I renamed .eslintrc to something else which is not findable by cra, like config.eslint.js. Then I changed all eslint scripts in package.json to eslint -c config.eslint.js ...rest of params. The only problem I have now is that my configuration is not working in my editor too, I mean fix eslint errors on save is not working and I should change the configuration file for my editor too. It may not worth it to change all of these also.
In my case I am using coc-eslint and coc-nvim which can be configured to use the new config file as explained here: url.

abraidwood added a commit to abraidwood/create-react-app that referenced this issue Oct 27, 2020
@abraidwood
Copy link

@abraidwood abraidwood commented Oct 27, 2020

It feels like webpack.config.js should honour the SKIP_PREFLIGHT_CHECK environment variable something like this abraidwood@39452df

abraidwood added a commit to abraidwood/create-react-app that referenced this issue Oct 27, 2020
@fredefl
Copy link

@fredefl fredefl commented Oct 27, 2020

In my case, the errors I got after upgrading were in files that are not included in the build - and thus weren't linted in v3.4.0 but somehow are in v4.0.0. Maybe this is what you're seeing too?

@paulopacitti
Copy link

@paulopacitti paulopacitti commented Oct 28, 2020

I encountered this issue yesterday 😕 Spend hours trying to understand what was going on. I had to downgrade to v3.4.4, now it's working fine I wish this issue can be fixed soon

@unframework
Copy link

@unframework unframework commented Nov 3, 2020

Just want to post some extra context for anyone reading:

Originally CRA used its own config for linting files during build and did not consult main project eslintrc file at all. This was useful because a lot of the rules caught not just style issues but actual runtime bugs (e.g. rules-of-hooks). Also, one could still tell CRA to use project's main lint config via the EXTEND_ESLINT option.

Then in CRA v4.0 that was consolidated (see #9587) - now EXTEND_ESLINT is the default. That removed a lot of ambiguity but now stylistic rule errors block dev build, which is what this thread is all about.

This is actually not such a big issue for initial CRA lint setup (since most rules in eslint-config-react-app are set to "warn") but for those of us using config presets like eslint-config-airbnb, etc, most of the rules are set to "error", and this starts to interrupt workflow. So, when people started upgrading to CRA v4.0, this behaviour began to be very noticeable.

Hopefully the above provides some background information for why this came up at this point in time.

@abraidwood
Copy link

@abraidwood abraidwood commented Nov 4, 2020

I'd like to be able to disable CRA running of eslint completely. I already have a git precommit hook that runs run eslint & prettier fixing for code layout then will block commit if further rules are failing. I also have linting running on CI as a merge-blocking step (in case anyone disables the precommit hook).

I feel like CRA is dictating and restricting workflow with this change & that is unhelpful and beyond its scope.

I also don't see why it is enabled when SKIP_PREFLIGHT_CHECK is true. I'm also confused about that flag's purpose as in the code it looks like it disables another, separate path to eslint & I don't understand why there are two.

@gaearon
Copy link
Member

@gaearon gaearon commented Nov 12, 2020

Do we have a confirmation that this only happens for people with custom configs?

@rswerve
Copy link

@rswerve rswerve commented Nov 12, 2020

Do we have a confirmation that this only happens for people with custom configs?

If I get rid of .eslintrc.js, I see the "old" behavior of compiling with warnings. If I keep .eslintrc.js, any rules not set to "warn" in that file will result in "Failed to compile" with a listing of broken rules (the two rules I tested were no-unused-vars and no-debugger).

@Poyoman39
Copy link
Author

@Poyoman39 Poyoman39 commented Nov 13, 2020

Do we have a confirmation that this only happens for people with custom configs?

I confirm for me too. In my case it come from my air-bnb extended eslint

@individual11
Copy link

@individual11 individual11 commented Nov 13, 2020

Same for me as above. I commented out the air-bnb in the extend array, and things started working again.

@chrisl8
Copy link

@chrisl8 chrisl8 commented Nov 17, 2020

+1 here on confirming the relationship to existing rules.

I have always had 'airbnb' in my extends list. It has been there for years on dozens of projects with CRA 3.

With CRA 4, it causes dev build to fail on unused variables. Removing airbnb fixes it.

@LuoTuxiu
Copy link

@LuoTuxiu LuoTuxiu commented Nov 18, 2020

I also get no-console complier error, I fixed it by remove the old eslintrc.js and overrides rules in package.json:

  "eslintConfig": {
    "extends": [],
    "overrides": [
      {
        "files": [
          "**/*.ts?(x)"
        ],
        "rules": {
          "no-console": "off"
        }
      }
    ]
  },
@mryechkin
Copy link

@mryechkin mryechkin commented Nov 20, 2020

I'm also seeing this issue - unfortunately none of the suggested solutions work. I had briefly thought that @LuoTuxiu's suggestion worked, but that's because I forgot to include the offending file 🤦‍♂️

kelvinfan001 added a commit to kelvinfan001/findalicious that referenced this issue Dec 29, 2020
To accommodate the eslint 7.x, react-scripts have also been updated
to 4.x. However, 4.x introduced some breaking changes, most notably
facebook/create-react-app#9887, which is
preventing react from compiling due to linting errors in our own
eslintrc.js (particularly airbnb). Disable airbnb for now so we can
at least compile.

In @avatarneil's soon-to-come mega PR that fixes all linting issues,
we can add airbnb back.
@fernandaad
Copy link

@fernandaad fernandaad commented Jan 4, 2021

Since the issue fix is scheduled for 4.0.2 I used patch-package to create a temporary workaround. You can do the following to fix the issue:

  1. Follow the steps at patch-package set-up section

  2. In node_modules\react-scripts\config\webpack.config.js you can change the ESLintPlugin config to allow compiling when has eslint erros adding failOnError as false (I also added emitWarning as true to see errors in console):

     new ESLintPlugin({
        // Plugin options
        failOnError: false, // add this line
        emitWarning: true,  // add this line
        extensions: ['js', 'mjs', 'jsx', 'ts', 'tsx'],
        formatter: require.resolve('react-dev-utils/eslintFormatter'),
        eslintPath: require.resolve('eslint'),
        context: paths.appSrc,
        cache: true,
        // ESLint class options
        cwd: paths.appPath,
        resolvePluginsRelativeTo: __dirname,
        baseConfig: {
          extends: [require.resolve('eslint-config-react-app/base')],
          rules: {
            ...(!hasJsxRuntime && {
              'react/react-in-jsx-scope': 'error',
            }),
          },
        },
      }),
  1. Then run npx patch-package react-scripts. This will create a patch to your project that whenever someone installs dependencies, it will change the webpack.config.js file at react-scripts folder.
@ashfaqnisar
Copy link

@ashfaqnisar ashfaqnisar commented Jan 6, 2021

Since the issue fix is scheduled for 4.0.2 I used patch-package to create a temporary workaround. You can do the following to fix the issue:

  1. Follow the steps at patch-package set-up section
  2. In node_modules\react-scripts\config\webpack.config.js you can change the ESLintPlugin config to allow compiling when has eslint erros adding failOnError as false (I also added emitWarning as true to see errors in console):
     new ESLintPlugin({
        // Plugin options
        failOnError: false, // add this line
        emitWarning: true,  // add this line
        extensions: ['js', 'mjs', 'jsx', 'ts', 'tsx'],
        formatter: require.resolve('react-dev-utils/eslintFormatter'),
        eslintPath: require.resolve('eslint'),
        context: paths.appSrc,
        cache: true,
        // ESLint class options
        cwd: paths.appPath,
        resolvePluginsRelativeTo: __dirname,
        baseConfig: {
          extends: [require.resolve('eslint-config-react-app/base')],
          rules: {
            ...(!hasJsxRuntime && {
              'react/react-in-jsx-scope': 'error',
            }),
          },
        },
      }),
  1. Then run npx patch-package react-scripts. This will create a patch to your project that whenever someone installs dependencies, it will change the webpack.config.js file at react-scripts folder.

Thank you very much @fernandaad, this helped a lot!

@mrmckeb
Copy link
Collaborator

@mrmckeb mrmckeb commented Jan 6, 2021

Hi all, we've created a discussion around this - as we do plan to make changes soon. Please let us know what you'd like to prioritise, or if we've missed something.

See #10344

jmatijas pushed a commit to jmatijas/tasks-app that referenced this issue Jan 26, 2021
Workaround is to downgrade react-scripts from 4.0.0. to 3.x.x and to remove devDepenencies for eslint.

After this commit is pulled from git, it requires removing node_modules dir and npm install.
deepcoderai added a commit to deepcoderai/cabana that referenced this issue Jan 26, 2021
- node-sass -> sass
- craco 6.0.0
- react 17.0.1
- react-scripts 4.0.1 (remove eslint-config-airbnb see facebook/create-react-app#9887 (comment) reintegrate later ?)
@vincentdesmares
Copy link

@vincentdesmares vincentdesmares commented Jan 26, 2021

In our setup it looks like our .eslintrc.json is ignored by the build. Yet, it is properly applied in VSCode.

duhdugg added a commit to duhdugg/preaction-cms that referenced this issue Jan 26, 2021
 fix tests
🔨 use craco for react-scripts workaround: facebook/create-react-app#9887
@RiverTwilight
Copy link

@RiverTwilight RiverTwilight commented Feb 1, 2021

I also get no-console complier error, I fixed it by remove the old eslintrc.js and overrides rules in package.json:

  "eslintConfig": {
    "extends": [],
    "overrides": [
      {
        "files": [
          "**/*.ts?(x)"
        ],
        "rules": {
          "no-console": "off"
        }
      }
    ]
  },

Thanks, but I fixed it after adding something to the extends field based on your solution.

"extends": [
	"react-app",
	"react-app/jest"
],
@iansu iansu closed this in #10170 Feb 3, 2021
@mrmckeb
Copy link
Collaborator

@mrmckeb mrmckeb commented Feb 3, 2021

Hi everyone, you can now change this behaviour as of the latest react-scripts release, 4.0.2. There are two new flags, ESLINT_NO_DEV_ERRORS and DISABLE_ESLINT_PLUGIN.

Please see the PR #10170 or docs for more info!
https://create-react-app.dev/docs/advanced-configuration/

@KyleRAnderson
Copy link

@KyleRAnderson KyleRAnderson commented Feb 4, 2021

Forgive me if this sounds naiive, I'm a little new to create-react-app, but does the configuration text mentioned in the CRA ESLint documentation go in .eslintrc.<whatever> or somewhere else? I'm a little confused because it doesn't quite look like that's an eslint config file of its own, with the "eslintConfig" key being at the root.

@KyleRAnderson
Copy link

@KyleRAnderson KyleRAnderson commented Feb 4, 2021

Forgive me if this sounds naiive, I'm a little new to create-react-app, but does the configuration text mentioned in the CRA ESLint documentation go in .eslintrc.<whatever> or somewhere else? I'm a little confused because it doesn't quite look like that's an eslint config file of its own, with the "eslintConfig" key being at the root.

Nevermind, I figured out it goes in package.json, I guess that makes sense.

@mrmckeb
Copy link
Collaborator

@mrmckeb mrmckeb commented Feb 4, 2021

@KyleRAnderson it can go in the root too if you prefer, as a valid ESLint config file.

Just keep in mind that the behaviour as of v4.0 is that your ESLint config is used within the Webpack ESLint Plugin, which means that errors will show in the error overlay (which is what this issue, and solution are about).

@ashfaqnisar
Copy link

@ashfaqnisar ashfaqnisar commented Feb 4, 2021

Awesome Work

Hi everyone, you can now change this behaviour as of the latest react-scripts release, 4.0.2. There are two new flags, ESLINT_NO_DEV_ERRORS and DISABLE_ESLINT_PLUGIN.

Please see the PR #10170 or docs for more info!
https://create-react-app.dev/docs/advanced-configuration/

Truly Awesome Work 👏🔥🔥

@KyleRAnderson
Copy link

@KyleRAnderson KyleRAnderson commented Feb 5, 2021

@KyleRAnderson it can go in the root too if you prefer, as a valid ESLint config file.

Just keep in mind that the behaviour as of v4.0 is that your ESLint config is used within the Webpack ESLint Plugin, which means that errors will show in the error overlay (which is what this issue, and solution are about).

Thank-you for clarifying, that makes a lot of sense, and thank-you for resolving this issue. It was something I was facing as well 😃

@rus-yurchenko
Copy link

@rus-yurchenko rus-yurchenko commented Feb 5, 2021

@mrmckeb possible related issue #10509

@CarlosHdez
Copy link

@CarlosHdez CarlosHdez commented Feb 5, 2021

I'm also a bit new to CRA so I'm having troubles with this, I'm adding the new rule to my .env as says on the docs but when I try to run the dev server I keep geetting the errors that prevent the build. This is only happening with ESLINT_NO_DEV_ERRORS using DISABLE_ESLINT_PLUGIN does work. Thanks for the help

@cjones26
Copy link

@cjones26 cjones26 commented Feb 10, 2021

@CarlosHdez -- I'm seeing the exact same behavior.

I have two files, .env and .env.production which look like the following:

.env

SKIP_PREFLIGHT_CHECK=true
ESLINT_NO_DEV_ERRORS=true

.env.production

SKIP_PREFLIGHT_CHECK=true
ESLINT_NO_DEV_ERRORS=true
PUBLIC_URL=/my-app

Unfortunately the ESLINT_NO_DEV_ERRORS does not work, while DISABLE_ESLINT_PLUGIN does.

@mrmckeb
Copy link
Collaborator

@mrmckeb mrmckeb commented Feb 11, 2021

Thanks for reporting this @cjones26 and @CarlosHdez. Are either of you able to provide a reproduction of this?

Using react-scripts@4.0.2 I'm able to confirm that this works with a few errors/warnings, but it could be to do with the number of errors... Or something else might be at play.

In this example, I have one error, one warning. With this flag, both are now warnings
image

@rus-yurchenko
Copy link

@rus-yurchenko rus-yurchenko commented Feb 11, 2021

@mrmckeb for some reason, I can see that all errors became warnings with the new flag in the terminal, but it still throws the error on the app error overlay. That's the point of this issue.

@mrmckeb
Copy link
Collaborator

@mrmckeb mrmckeb commented Feb 11, 2021

@rus-yurchenko, this was supposed to fix that, I'm sorry if you aren't seeing it fixed.

Can you please share a screenshot, or some more details? A reproduction would be ideal :)

@miguelCT
Copy link

@miguelCT miguelCT commented Feb 11, 2021

@mrmckeb I left a comment with a Codesandbox Container example where the problem metioned by @rus-yurchenko can be reproduced. You can see it here

@mrmckeb
Copy link
Collaborator

@mrmckeb mrmckeb commented Feb 11, 2021

Thanks @miguelCT. Interestingly I can reproduce with only some errors, not all (maybe only TypeScript-related errors).

I'll keep investigating. I see they also made changes to the way this works in the plugin itself, which I also need to dig into:
https://github.com/webpack-contrib/eslint-webpack-plugin/blob/master/CHANGELOG.md#250-2021-02-04

@cjones26
Copy link

@cjones26 cjones26 commented Feb 11, 2021

Thanks @miguelCT. Interestingly I can reproduce with only some errors, not all (maybe only TypeScript-related errors).

I'll keep investigating. I see they also made changes to the way this works in the plugin itself, which I also need to dig into:
https://github.com/webpack-contrib/eslint-webpack-plugin/blob/master/CHANGELOG.md#250-2021-02-04

Would it still be helpful if we provided an example, or is the example provided by @miguelCT sufficient?

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

Successfully merging a pull request may close this issue.