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 · 90 comments · Fixed by #10170 or #10590
Closed

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

Poyoman39 opened this issue Oct 24, 2020 · 90 comments · Fixed by #10170 or #10590

Comments

@Poyoman39
Copy link

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)
@dev-bjoern
Copy link

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

@mstykow
Copy link

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 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 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

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

@rus-yurchenko
Copy link

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

Try with caching, again add
cache: true,

in ESLintPlugin options.

@Saibamen
Copy link

Saibamen commented Oct 26, 2020

Caching fixed in this PR: #9911

@asdfasdfasdfasdfff
Copy link

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 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 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

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 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

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
Contributor

gaearon commented Nov 12, 2020

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

@rswerve
Copy link

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

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

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

@chrisl8
Copy link

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 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

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 🤦‍♂️

@channyeintun
Copy link

I set SKIP_PREFLIGHT_CHECK=true and
ESLINT_NO_DEV_ERRORS=true in .env file.

@ashfaqnisar
Copy link

@channyeintun can you provide a reproduction of the issue ?

@ehaynes99
Copy link

no-undef is a syntax error, not a linting error, so webpack fails to compile the module at all. This wouldn't have rendered under react-scripts@3.4.1 either.

If you fix your no-undef problems, but leave the no-unused-vars problem, it would fail under 4.0.2 but render with 4.0.3

@RiverTwilight
Copy link

RiverTwilight commented Feb 24, 2021 via email

@mrmckeb
Copy link
Contributor

mrmckeb commented Feb 24, 2021

@channyeintun It looks like you're only seeing the output in the console, so you're not seeing dev errors. Is it showing the overlay?

@channyeintun
Copy link

@mrmckeb
Thanks, yesterday, the problem exists. When I try today the create-react-app, there is no more errors and warnings. I think you guys solved it already.

@ehaynes99
There was no syntax errors.
For example, the state is actually ok. Just linting had some problems. It is now ok. Thanks
16141968761232717854448274440135

@maksim-chekrishov
Copy link

eject + emitError=false

works for me

@channyeintun
Copy link

eject + emitError=false

works for me

Now no error exists. It's fixed. No need to configure anything. Just execute create-react-app project-name.

@mrmckeb
Copy link
Contributor

mrmckeb commented Feb 25, 2021

@maksim-chekrishov We don't recommend ejecting, as you'll no longer receive updates from us - including security updates.

Try to eject only if you really need to modify the scripts/configs yourself and want to manage the configs/upgrades for yourself.

@vincentdesmares
Copy link

Hi @mrmckeb,

I'm not sure if the problem is related but upgrading to React 4.0.3 from 3.x gave us new errors blocking the build, same as the author of this thread.

While I'm not against blocking the build I don't understand why I cannot modify those errors in our eslintrc.json file.

It looks like upgrading to 4.X now ignore our local configuration. For exemple, adding "@typescript-eslint/ban-ts-ignore": "off", successfully removes the error from VScode but the build is still failing the same way.

@mrmckeb
Copy link
Contributor

mrmckeb commented Feb 26, 2021

Hi @vincentdesmares, that sounds like a setup issue - as it definitely shouldn't behave that way - but it may be a bug of course! Can you share a reproduction? I can investigate further :)

@vincentdesmares
Copy link

I will try to reproduce the issue in a standalone repository.

I'm very tempted to just trash my previous config and start clean but right now if I want to configure my app, the only reference that I found is https://create-react-app.dev/docs/setting-up-your-editor and the emplacement of the configuration is not explicitly specified.

What's the available configurations?

@mrmckeb
Copy link
Contributor

mrmckeb commented Feb 26, 2021

It should be anything that ESLint supports: https://eslint.org/docs/2.0.0/user-guide/configuring#configuration-file-formats

Just make sure it's not defined in your package.json too - as that's where we place it by default.

@stclairdaniel
Copy link

I'm still getting this issue in 4.0.3 -

Failed to compile.

src/scripts/components/App.tsx
  Line 7:8:     'Loading' is defined but never used                            @typescript-eslint/no-unused-vars

@anushbmx
Copy link

@stclairdaniel add ESLINT_NO_DEV_ERRORS=true in your .env, it will skip these errors in development

@Nerdsie
Copy link

Nerdsie commented Jun 4, 2021

This still seems like an issue when building. ESLINT_NO_DEV_ERRORS=true fixes when simply running for development, but when building the build still fails on lint errors

@rswerve
Copy link

rswerve commented Jun 4, 2021

This still seems like an issue when building. ESLINT_NO_DEV_ERRORS=true fixes when simply running for development, but when building the build still fails on lint errors

That’s the expected behavior. It’s the DEV in NO_DEV_ERRORS. If you don’t want a particular error to break your build, you can turn off the specific linting rule in your eslint config.

@Nerdsie
Copy link

Nerdsie commented Jun 4, 2021

This still seems like an issue when building. ESLINT_NO_DEV_ERRORS=true fixes when simply running for development, but when building the build still fails on lint errors

That’s the expected behavior. It’s the DEV in NO_DEV_ERRORS. If you don’t want a particular error to break your build, you can turn off the specific linting rule in your eslint config.

Is there no build equivalent of ESLINT_NO_DEV_ERRORS? I don't want ANY lint errors to break my build, it's a style check, bad style should not be a reason for my deployment to suddenly be not building. I shouldn't have to go through and change all my lint rules to warning instead of errors, especially because then our CI workflow won't notify us of the lint "failure"

@rswerve
Copy link

rswerve commented Jun 4, 2021

This still seems like an issue when building. ESLINT_NO_DEV_ERRORS=true fixes when simply running for development, but when building the build still fails on lint errors

That’s the expected behavior. It’s the DEV in NO_DEV_ERRORS. If you don’t want a particular error to break your build, you can turn off the specific linting rule in your eslint config.

Is there no build equivalent of ESLINT_NO_DEV_ERRORS? I don't want ANY lint errors to break my build, it's a style check, bad style should not be a reason for my deployment to suddenly be not building. I shouldn't have to go through and change all my lint rules to warning instead of errors, especially because then our CI workflow won't notify us of the lint "failure"

@Nerdsie There is. Check out this PR. The environment variable DISABLE_ESLINT_PLUGIN set to true will do what you want.

@Nerdsie
Copy link

Nerdsie commented Jun 4, 2021

@rswerve Thanks, just what I need. It still feels weird to me to have to disable (both in prod and dev, via seperate options) build breaking on lint errors. Are people using eslint in such a way that lint errors catch more than just deviation from preferred styling?

@rswerve
Copy link

rswerve commented Jun 4, 2021

@rswerve Thanks, just what I need. It still feels weird to me to have to disable (both in prod and dev, via seperate options) build breaking on lint errors. Are people using eslint in such a way that lint errors catch more than just deviation from preferred styling?

@Nerdsie Yes. For example, to enforce the Rules of Hooks. If someone wants to break one of those rules, we want them to be aware they're doing it and disable it deliberately, so we can see it in code review. And even with styling, on a team you're either enforcing consistent styles or you're not; we find it's less to think about if we are, so we break the build (this almost never happens, because everyone runs Prettier locally).

kwajiehao added a commit to isomerpages/isomercms-frontend that referenced this issue Jun 7, 2021
Currently, when we attempt to start our react app, create-react-app
fails to compile when there are any eslint or prettier errors. The
reason for this is documented in this github issue: facebook/create-react-app#9887 (comment)

This issue is fixed with react-scripts v4.0.2 with the use of an
ESLINT_NO_DEV_ERRORS env var (this is documented under advanced
configuration here: https://create-react-app.dev/docs/advanced-configuration/)

This commit adds the ESLINT_NO_DEV_ERRORS env var to our env-example
file. We will need to separately add it to our production and staging
environments too.
kwajiehao added a commit to isomerpages/isomercms-frontend that referenced this issue Jun 7, 2021
Currently, when we attempt to start our react app, create-react-app
fails to compile when there are any eslint or prettier errors. The
reason for this is documented in this github issue: facebook/create-react-app#9887 (comment)

This issue is fixed with react-scripts v4.0.2 with the use of an
ESLINT_NO_DEV_ERRORS env var (this is documented under advanced
configuration here: https://create-react-app.dev/docs/advanced-configuration/)

This commit adds the ESLINT_NO_DEV_ERRORS env var to our env-example
file. We will need to separately add it to our production and staging
environments too.
@soullivaneuh
Copy link

Confirming @rswerve solution, here is my build npm script:

"build": "npx tsc --noEmit && DISABLE_ESLINT_PLUGIN=true CI=false react-scripts build",

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