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

Cannot use .eslintignore for generated files #7339

Closed
danielkcz opened this issue Jul 8, 2019 · 9 comments
Closed

Cannot use .eslintignore for generated files #7339

danielkcz opened this issue Jul 8, 2019 · 9 comments
Milestone

Comments

@danielkcz
Copy link

danielkcz commented Jul 8, 2019

Describe the bug

When running yarn start, the ESLint errors are shown from files I have ignored with .eslintignore and the app won't start.

Did you try recovering your dependencies?

Reproducible in a fresh new project.

Which terms did you search for in User Guide?

eslintignore (yielded #6871)

Environment

  System:
    OS: Windows 10
    CPU: (8) x64 Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
  Binaries:
    Node: 10.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.16.0 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: 44.17763.1.0
    Internet Explorer: 11.0.17763.1
  npmPackages:
    react: ^16.8.6 => 16.8.6
    react-dom: ^16.8.6 => 16.8.6
    react-scripts: 3.0.1 => 3.0.1
  npmGlobalPackages:
    create-react-app: Not Found

Expected behavior

To ignore specified files from ESLint checks and start the app. Talking here mostly about generated files which we cannot just modify with the comment to disable eslint. The best course of action is to just ignore these.

Quite an exhaustive issue #6871 talks about customizing ESLint config, but not much of the mention of utilizing .eslintignore which isn't exactly huge customization and shouldn't be prohibited like that.

I can use a workaround with cra-override, but I don't want to disable a conflicting rule for a whole project when the only a bunch of generated files is affected.

Actual behavior

image

I have VSCode properly working with ESLint in the project. The specified file is correctly ignored there.

Reproducible demo

https://github.com/FredyC/cra-eslint-ignore

@heyimalex
Copy link
Contributor

Slightly related to #7295. I'll look into what it would take to support eslintignore and bring it up at the next maintainers meeting? I'm not sure if there's a reason we don't support it or if it just hasn't been implemented.

@heyimalex
Copy link
Contributor

It looks like this was intentional via #2115, and it would be breaking so it's probably not going to change. There's more discussion in #2339. Sorry!

@danielkcz
Copy link
Author

danielkcz commented Jul 9, 2019

@heyimalex Well, that's 2 years old and I don't see any discussion except "we don't do that". What if some views have changed on the problem?

@Timer Please, what's your take on this after that time? And can you give it a serious thought and explain some actual reasoning? So far it seems it's just "inconvenient" and nothing else.

It's hard to believe that such a trivial problem needs hard solutions. I do understand why you don't want to support .eslintrc, way too much customization. But, ignoring files from linting is hardly called customization.

Besides, it would kinda solve #6871 and people could disable ESlint for a whole project if they really want.

@bugzpodder
Copy link

I think it makes sense to re-evaluate whether this is viable, given that we are working towards allowing custom eslint configs. However it is a breaking change so it would only be included in a major release that might be a while away.

@bugzpodder bugzpodder reopened this Jul 9, 2019
@bugzpodder bugzpodder added this to the 4.0 milestone Jul 9, 2019
@danielkcz
Copy link
Author

danielkcz commented Jul 9, 2019

I am not entirely sure how is it breaking considering its opt-in feature, but I am fine with it either way. Thanks for reopening.

@silverwind
Copy link

I too suggested removing this pesky ignore: false in the webpack config in #7036. My use case is for ignoring manually vendored JS libs in src.

Change should be fully backwards compatible.

@nphmuller
Copy link

nphmuller commented Sep 13, 2019

I would really love to use an .eslintignore file for copied over dependencies (like scripts). These kind of which I want to keep as much as possible as the original, so I can easily implement updates.
The best way would be to fork the script and make my own npm package, but that would be pretty overkill for most of my smaller projects.

Maybe this can be implemented without breaking backwards compatibility by requiring an additional env variable to be set? Maybe something like EXTEND_ESLINT_ALLOW_ESLINTIGNORE?

Edit: Since extending the ESLint config is still experimental maybe a breaking change is okay at this point?

@joejordan
Copy link

Came across this thread after scratching my head as to why I could customize eslint in my CRA app to my heart's desire using .eslintrc.js, but then being unable to compile due to .eslintignore being ignored.

I think an env variable as suggested by @nphmuller is a good solution to offer this needed functionality before the long and distant CRA 4.0 is released.

@silverwind
Copy link

This should be resolved in 3.2.0 with 6f5221c.

@lock lock bot locked and limited conversation to collaborators Oct 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants