-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Support ESLint configure or disabling ESLint (discussion) #3886
Comments
Could you please extract specific pain points from the post in the form of a bullet list? I don’t want to get into an argument about whether configuration is necessary/useful or not. I just want to see a specific list of perceived pain points, phrased in short sentences. For example:
If you phrase your other pain points in a concise, specific way (instead of going into the philosophy of configuration) we can addesss them better. |
Some of the things I can't reveal specificly because of confidentiality. But, I talked about specifics even if it's not "correct". For example:
I hope it's clearer (I believe it isn't because for most cases I can't reveal code). |
@oriSomething @gaearon Adding to the list:
❯ yarn lint
yarn run v1.3.2
$ eslint src/**/*.js
Cannot find module 'eslint-config-react-app'
Referenced from: /dev/my-project/.eslintrc.json
Error: Cannot find module 'eslint-config-react-app'
Referenced from: /dev/my-project/.eslintrc.json
at ModuleResolver.resolve (/dev/my-project/node_modules/eslint/lib/util/module-resolver.js:74:19)
...
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Proposal
The way I see it, enforcing "zero config" concepts on such opinionated/versatile tools like linters only harms the experience |
I keep a clean repo, and after upgrading: the no-unused-vars rule splits out a lot of warnings and is very annoying. Usually when I have an event handler, I don't need the event or when I do array.map i just want the index. There are tons of callsites and I do not enjoy putting an eslint-disable comment for these intentional use cases. |
CRA is opinionated. If you wanna ESLint configurations meeting your need, try |
It's pretty terrible that the CRA repro itself has to litter eslint-disable statements everywhere to satisfy its own eslint rules. https://github.com/facebook/create-react-app/search?utf8=%E2%9C%93&q=no-unused-vars&type= |
Also terrible: nobody answering this thread. |
Linting is imho CRA's biggest weak point at the moment. My biggest issue with current CRA's handling of eslint is that it strictly enforces eslint as errors during the dev cycle, making it really annoying to quickly try stuff out while debugging. I always end up completely disabling CRA's eslint during development and rely on the linting of my editor of choice, which is a shame because linting with CRA shouldn't be so cringe-worthy. I like @nfantone's proposal. |
I have a project with some customized rules: (...)
"rules": {
"react/jsx-filename-extension": [1, {"extensions": [".js"]}],
"react/jsx-no-duplicate-props": [1, { "ignoreCase": false }],
"prettier/prettier": ["error", {
"arrowParens": "always",
"singleQuote": true,
"trailingComma": "es5"
}]
},
(...) I am using Material-UI-Next, where you can get some components like this (notice the two <TextField
type="number"
name="sumInsured"
label="Insured sum"
value={sumInsured}
inputProps={{ min: 0, max: 100000 }}
InputProps={{ disableUnderline: true }}
onChange={handleChange}
className={styles.question}
/> My npm scripts, and editor pick these rules correctly, hence not showing a warning on these components, but when running This is very annoying, because I can't event create builds when |
+1 for not being able to use lint-staged on my mono-repo, if my frontend has create-react-app. I wish there was a way to give precedence to configurations higher up in the tree. |
@andrewnaeve I have lint-staged and husky setup in my monorepo:
and in each of the package.json in a workspace I have the standard lint-staged setup with concurrency: false and no husky installation. |
@bugzpodder Wow thanks! |
We're finding more and more rules which fall outside CRA's "errors only" and Prettier's "styles only", meaning we have no way to make use of them. e.g prettier/prettier#949 @gaearon what would you recommend here, e.g. submit a pull request switching this rule on in the next branch? The issue here is that there's no fallback if the PR Is rejected |
Workaround to use one's own "scripts": {
"postinstall": "sed -i -E 's#\\[.*eslint-config-react-app.*\\]#['\\'$PWD/.eslintrc\\'']#g' node_modules/react-scripts/config/webpack.config.*.js"
} |
I have my own .eslintrc which works in conjunction with CRA's eslintrc and it works pretty well for the past year: |
So here is what I think CRA should do:
This should make everyone happy. Users get to extend the configuration and can install plugins and webpack and CLI/Editor linting are ran with this same linting configuration. |
What @silverwind said except use https://github.com/davidtheclark/cosmiconfig so I can delete that But yeah, CRA's linting rules are too weak to be useful. I think the purpose of CRA should be to obfuscate Webpack junk while still exposing nice things like ESlint and Babel config. I was never really clear on why these 2 configs aren't exposed.
^ Are these going to outnumber the really valid "Can I please configure my code style...?" or "Can I add that Babel decorator plugin?" issues that get raised here every day? Also, the community can help steer people to the right places for their issues, or hell, even just help people. |
|
We enforce no style rules, if you find one we enforce by accident, please file a separate issue to have it disabled. We're not ready to let people configure ESLint to apply any rules they desire yet, sorry! |
@Timer I would like to point out one very frustrating point - a lot of projects utilize css libraries like Bootstrap or Semantic-UI. When using a third party framework you have to follow their rules, one example:
The problem here is the
I realize that the html snippet is useless but In a more specific example I'm using When I'm sure folks can place many many more examples here of where |
from a mountain top: Prettier is not a linter. code quality rules !== formatting rules. This is really still an issue (since 2016)? Linting should be done at compile and re-compile and OF COURSE every individual and/or organization will have different code quality rules! How does enforcing your own eslint config help users? Hint: it doesn't. |
So is there a hack-ish way to use my own .eslintrc with a newly-created CRA project? |
@hiquest you currently have two options: Eject, or: Use react-app-rewired and put something like this in your
^ haven't tested that code, but you get the idea |
@russellr86 you could also easily do it with craco, it's quite simple, all you need to do is 1- Create a 2- Tell
You can find a recipe here: https://github.com/sharegate/craco/tree/master/recipes/use-an-eslintrc-config-file |
@crobinson42 that eslint console output is correct.
The other way to fix this is to use buttons instead of links.
It's an accessibility requirement that applies to any use of HTML whether or not React is used. |
Maybe we should remove |
Sorry for removing the Github template, it was irrelevant. Anyway:
As continuation of my comment from #3815
I know this issue discussed in the past in many different issues, and yet, I think we should revisit the support of custom ESLint config by user or at least the ability to remove it so we can use our configuration or even in cases where users don't feel they need ESLint.
Good part with current state
Pain points with current state
Currently the only option to really control the configuration of ESLint is by eject or by fork. In both cases we lose the charm of using
react-scripts
which makes most things to be more convenient and tools that are updated automagically between versions of packages such aswebpack
endESLint
as well.But lack of controlling the configuration can cause a lot more issues, which makes the ESLint part in
create-react-app
to be a pain for development.The
eslint-config-react-app
does contains stylistic rules, even tough they're not from the "prettier" kind. For example:default-case
rule forces me to put emptydefault:
case even when it's an empty and unneeded, since in my project the convention it to do the "default" after the "switch-case block" or in cases whendefault
is really unneeded.It's easy to say just add
/* eslint-ignore default-case */
, but it becoming more of a "file template" than some exception that happens "here and there".Moreover, even most consensus rules such as
eqeqeq
might not feet to any project, and will be more of stylistic choice rather than the right one. For example, I worked on a project where I've used Flow and have a lot of cases where==
was more than a necessary to make code readable, since when you're type isstring | null | undefined
and you need to make lots of compares things likea == b
was much better thana === b || (a == null && b == null)
(I've simplified the "If statement"). Flow can make sure it's notnumber
compared tostring
(And I know, it's an edge case, but it was only for a demonstration).Plugins are very useful, but when I can't config, it means no real use of plugins. Because, on the browser I'll have one configuration and on then IDE / CLI I'll have other configuration, which is a pain. There some cases you'll need to remove some ESLint rule because some plugin gives you a better rule which might conflict with the original rule. But you can't configure, so warnings / errors in the browser will be leave you with conflicts.
And The actual use of plugins is discourage since, as I said before, I'll get in the browser different result from what happens in the IDE / CLI.
Half related issue: #1345 (Why should I need to wait to the support or be forced to the plugin if I don't need?)
Globals are also a big issue, If you forced to use third party code from a CDN it means you'll have a lot of
/* globals myAwesomeGlobal */
in your files. And please don't tell me to usewindow
orglobal
, It's a temporary work around not a solution.In many cases when I'll want to remove the "global" since we stop to use some third party service, I won't have an easy way to know it "truly happens". When I'm not the only one who work on the project. Using
window
orglobal
will only make it harder. Moreover, as browsers become more and more modern and some script will start use global aslet
orconst
the variable won't exist on thewindow
object (not to mention when a file should run also inside Worker / Node which only makes things more complicated for using globals).Moreover, there are libraries such as
Modernizr 3.0
that force me to generate a file tosrc
folder, that generated automatically. If I'll add it topublic
it will force another network request call and If I put it insrc
it forces me to add manually/* eslint-disable */
or start creating boilerplate scripts for this kind of code injection.Relates issue: #2339 , #2863
Don't get me wrong, I fully understand the project philosophy and admire the work that have been done. But, on the other hand you lock me with this project "configuration" for a linter with specific rules which makes it harder work for me than not having the linter from the first place.
Since there is no easy way to opt-out without ejecting/forking, it makes it harder as more conflicts with the
eslint-config-react-app
happens during project life time.Moreover, you actually do have configurations such as
"homepage"
. Even if it's not much. ESLint is one of the biggest pain points for me in thecreate-react-app
suite.Suggestion
My suggestion is to allow custom configure or at least to disable ESLint project wide.
Related issue for disable ESLint: #2157
Related issue for allow custom config: #2318
P.S. I can find more related issues to every issue I've wrote here, but I don't think it gives any value.
The text was updated successfully, but these errors were encountered: