-
-
Notifications
You must be signed in to change notification settings - Fork 27k
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
disable eslint #2157
Comments
You can’t. Are there particular rules you want to disable? Why? |
Or anyway to use our own eslintrc from npm start? problem is we have our own eslint settings and its different from CRA. |
There is a long discussion about this in #808 but TLDR is:
We know that the community is still accustomed to ESLint styling rules (not something ESLint was built for). This means that everyone will try to use their own configs which usually include a lot of styling rules. As a net result the browser and terminal ESLint integration becomes too noisy and obscures real problems behind stylistic nits. We are pushing the community towards replacing ESLint styling rules with tools like Prettier. Unfortunately it means inconvenience of disallowing any ESLint customization in the meantime. In the future, when we integrate Prettier into CRA by default and enforce its usage, we might enable ESLint rule customization. I hope this helps! |
@maxlibin If you want to enforce style with eslint, i'm quite happy with adding these scripts on my package.json and add a .eslintrc on the root of the project.
And maybe use lint as pre commit. In my experience, fixing style nits is time consuming without added benefits. So it's better to leave it off when you actually solving the problem, and automatically format it when checked to repo. |
So, I totally understand the arguments given, but we've stumbled upon a situation in which not being able to disable linting is becoming a significant headache. It's a pretty niche case, so I don't expect anything to be changed, but thought I'd mention it so that it can at least be "on the radar" of things people do. At Khan Academy we're using Create React App for prototyping. The goal of the project is to quickly test new ideas before building them "for real" in our monolithic repo. This project imports all of our existing modules. These files work perfectly, but fail a bunch of lint checks; specifically, The result is that the Chrome console is absolutely flooded with thousands and thousands of warnings. These warnings bury any user-specified logging, errors thrown, etc. Anyway. As I said, I recognize that this is an unconventional situation. Leaving it here for the record, but no action or response is needed or expected :) |
The solution to this is the same one as usual: instead of looking for a way to disable rules, file an issue and describe the specific use case where you think the rule doesn’t make sense. We are always open to relaxing rules if some of them don’t make sense for projects. 😉 In your particular example it sounds like setting |
To clarify, our point is not to be dogmatic. It’s to find a good subset of rules that work for everyone. Unfortunately we can’t do this if everyone makes escape hatches for themselves without reporting their scenarios. Having to go through a common process lets us create a really good ESLint config, even though it comes at the cost of having more discussion. |
Right, so I didn't really explain this very well. In other words, it's not that I think that the rules should be changed, because the rules do make sense (I do like the proposed change to As I said, I recognize that this is probably not a widespread concern! I share it simply so that you can be aware that this is a thing at least 1 team is doing, but I wouldn't want create-react-app to be made more complex to handle this problem unless it became clear that a lot of users had this issue (which I doubt). |
Yea, fair enough. In this case we optimize for the case of new code, and IMO we do the right thing by displaying them. If the main issue is with console spam in the browser for legacy code, we can limit the number of displayed warnings in the browser (e.g. to first five files). Would that help? |
Yeah, that would totally solve the issue for us. I don't want to burden anyone else with this though, happy to submit a PR for this! |
Added in #2327. |
Both changes should be out in |
Yay, thanks Dan! :D |
I have this method: currentPage() {
switch ( this.state.action ) {
case "projects":
if ( this.state.projects ) return <Projects items={this.state.projects.items} />;
break;
case "builds":
if ( this.state.projects ) return <Builds project={this.state.projects.currentProject} />;
break;
}
return <NotFound />;
} Which basically enforces a condition on each matched case and fallback to a This, of course, triggers this lint error:
Well, no, I won't duplicate the line returning I won't ask you to change create-react-app because of this, when I'll have enough of this warning, I'll just eject and remove the linter, but I had to let you know since you say you want to detect only logical mistakes and not styles. By the way, if we disregard coding styles, is there anything a linter can do that flowtype can't? |
I understand it is subjective, but in this case I think the warning is valuable. It is easy to make a mistake in code that relies on both Generally when I write code like this, I think how to restructure it for clearer and more explicit control flow. I know I’m not smart enough to come back to it after a few months and not to accidentally misunderstand it. Again, this is debatable, and your point is valid, but I do believe having this warning is a net win. If it’s common enough for you to eject, and you don’t agree it’s worth rewriting the code, well, let’s agree to disagree 😛 .
You don’t have to duplicate it. You can intentionally fall through. default:
// Fall through
break; This is enough to appease the linter. This tells the person reading your code (or you in a few months) that lack of |
Sure, there are tons of way I could refactor it, or even totally remove the method ("hey, why not use react-router instead?"), but refactoring legit code to satisfy linter always makes me roll eyes (and this is about as much as I'm annoyed - not a big deal). I just wanted to point out this was a case breaking the "not enforcing coding style" rule. So, I guess we mainly disagree on what is coding style and what is not :) That's ok, though, just wanted you to know, but I can live with it. Create-react-app rocks anyway, keep on the good work! |
Thanks!
I agree it's a thin line. Generally we err on the side of enforcing that potentially ambiguous code is rewritten more explicitly. Adding a |
I get you, this is a library, used by many, so you have to consider which errors will be the most common. In this case of "enforcing |
If i can't disable or tweak lint, how can I get around this?
|
can you move the |
thanks @viankakrisna , it was by accident I added a package to yarn and created a new package.json inside |
For anyone interested, in my environment (react+typescript) I can prevent tslint throwing messages in console just deleting all |
Is it possible to disable eslint during |
If you are ok with ejecting you can disable it directly from webpack config I think |
For my specific project ESLint ends up taking about 10 seconds. After ejecting and disabling the build goes down to a second. :( I really don't want ESLint, I have my own linting systems. :| |
oh...in CRA 2.1.1 how can i overwirte the esling rule .? i have own linting config. maybe i have ocd . :( |
how can i remove eslint in npm start?
The text was updated successfully, but these errors were encountered: