Skip to content

disable eslint #2157

Description

@maxlibin

how can i remove eslint in npm start?

Activity

  1. gaearon commented on May 15, 2017

    @gaearon
    Contributor

    You can’t. Are there particular rules you want to disable? Why?

  2. maxlibin commented on May 15, 2017

    @maxlibin
    Author

    Or anyway to use our own eslintrc from npm start? problem is we have our own eslint settings and its different from CRA.

  3. gaearon commented on May 15, 2017

    @gaearon
    Contributor

    There is a long discussion about this in #808 but TLDR is:

    • Our rules are specifically picked to not enforce style, and to only find logical mistakes.
    • We are open to making our rules work better for everyone (if you have specific ideas about adding or removing rules, please file issues!)
    • We think ESLint rules are a fundamentally flawed way to enforce coding style, and we suggest using Prettier for that instead.

    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!

  4. viankakrisna commented on May 15, 2017

    @viankakrisna
    Contributor

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

        "format": "prettier --trailing-comma es5 --single-quote --write 'src/*/*.js' 'src/*/!(node_modules)/**/*.js'",
        "lint": "npm run format && eslint --fix --config .eslintrc src"
    

    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.

  5. joshwcomeau commented on May 22, 2017

    @joshwcomeau
    Contributor

    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, no-use-before-define fails several times on almost every JSX file, because we define a style object (for use with Aphrodite) below the component that uses those styles.

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

  6. gaearon commented on May 22, 2017

    @gaearon
    Contributor

    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 variables: false option for no-use-before-define would solve the issue. I’m open to doing it.

  7. gaearon commented on May 22, 2017

    @gaearon
    Contributor

    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.

  8. gaearon commented on May 22, 2017

    @gaearon
    Contributor
  9. joshwcomeau commented on May 22, 2017

    @joshwcomeau
    Contributor

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

    Right, so I didn't really explain this very well. no-use-before-define is one of the most prominent ones, but it was just an example. Other linting checks fail, and to be honest they should fail: various jsx-a11y rules, strict for unnecessary "use strict", and jsx/pascal-case, which fails because in our main repo we do some weird thing to handle i18n translations with "elements" that look like <$_>, but get pre-compiled into a translated string.

    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 no-use-before-define, but I certainly wouldn't want the other rules to be changed!). In our case, the issue is the legacy code we're building on top of, but there's too much of it to change, at least for now.

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

  10. gaearon commented on May 22, 2017

    @gaearon
    Contributor

    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?

  11. joshwcomeau commented on May 22, 2017

    @joshwcomeau
    Contributor

    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!

  12. gaearon commented on May 22, 2017

    @gaearon
    Contributor

    Added in #2327.

  13. gaearon commented on May 22, 2017

    @gaearon
    Contributor

    Both changes should be out in react-scripts@1.0.5.

  14. joshwcomeau commented on May 22, 2017

    @joshwcomeau
    Contributor

    Both changes should be out in react-scripts@1.0.5.

    Yay, thanks Dan! :D

  15. oelmekki commented on Jun 29, 2017

    @oelmekki

    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 <NotFound /> component if app is not properly setup or if the required action does not exist.

    This, of course, triggers this lint error:

    Line 26:  Expected a default case  default-case
    

    Well, no, I won't duplicate the line returning <NotFound /> in a default case, since I need it anyway when condition in matched cases fail. I think this illustrates why linters are so annoying : wanting to prevent common errors, they force us to avoid perfectly legit, although unusual, code.

    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?

  16. gaearon commented on Jun 29, 2017

    @gaearon
    Contributor

    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 switch, if, and returns in different places for control flow. Maybe you won’t make mistakes with this, but many people will.

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

    Well, no, I won't duplicate the line returning in a default case, since I need it anyway when condition in matched cases fail

    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 return there was intentional, and not an accidental omission.

  17. oelmekki commented on Jun 29, 2017

    @oelmekki

    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!

  18. gaearon commented on Jun 29, 2017

    @gaearon
    Contributor

    Thanks!

    So, I guess we mainly disagree on what is coding style and what is not

    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 default case is generally valuable because it forces the person writing the code to think "what happens if someone adds another option/route/query". And thus prevents timebomb code that blows up when somebody extends it.

  19. oelmekki commented on Jun 29, 2017

    @oelmekki

    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 default: even with a fallback after the switch", I guess we can say it's a logical mistake consideration with a coding style side effect :)

  20. Andsbf commented on Aug 1, 2017

    @Andsbf

    If i can't disable or tweak lint, how can I get around this?

    Failed to compile.
    
    ./src/~/react-router-dom/es/index.js
      Line 3:   Import in body of module; reorder to top  import/first
      Line 5:   Import in body of module; reorder to top  import/first
      Line 7:   Import in body of module; reorder to top  import/first
      Line 9:   Import in body of module; reorder to top  import/first
      Line 11:  Import in body of module; reorder to top  import/first
      Line 13:  Import in body of module; reorder to top  import/first
      Line 15:  Import in body of module; reorder to top  import/first
      Line 17:  Import in body of module; reorder to top  import/first
      Line 19:  Import in body of module; reorder to top  import/first
      Line 21:  Import in body of module; reorder to top  import/first
      Line 23:  Import in body of module; reorder to top  import/first
      Line 25:  Import in body of module; reorder to top  import/first
    

    src/node_modules/react-router-dom/es/index.js

    import _BrowserRouter from './BrowserRouter';
    export { _BrowserRouter as BrowserRouter };
    import _HashRouter from './HashRouter';
    export { _HashRouter as HashRouter };
    import _Link from './Link';
    export { _Link as Link };
    import _MemoryRouter from './MemoryRouter';
    export { _MemoryRouter as MemoryRouter };
    import _NavLink from './NavLink';
    export { _NavLink as NavLink };
    import _Prompt from './Prompt';
    export { _Prompt as Prompt };
    import _Redirect from './Redirect';
    export { _Redirect as Redirect };
    import _Route from './Route';
    export { _Route as Route };
    import _Router from './Router';
    export { _Router as Router };
    import _StaticRouter from './StaticRouter';
    export { _StaticRouter as StaticRouter };
    import _Switch from './Switch';
    export { _Switch as Switch };
    import _matchPath from './matchPath';
    export { _matchPath as matchPath };
    import _withRouter from './withRouter';
    export { _withRouter as withRouter };
    
  21. viankakrisna commented on Aug 1, 2017

    @viankakrisna
    Contributor

    can you move the node_modules folder outside of src? It should be placed in the root (beside ./src) for dependencies. All files inside ./src should only contain user created code.

  22. Andsbf commented on Aug 1, 2017

    @Andsbf

    thanks @viankakrisna , it was by accident I added a package to yarn and created a new package.json inside src

  23. YagoLopez commented on Dec 2, 2017

    @YagoLopez

    For anyone interested, in my environment (react+typescript) I can prevent tslint throwing messages in console just deleting all tslint.json rules and leaving an empty object {}

  24. ChristosChristofidis commented on Aug 17, 2018

    @ChristosChristofidis

    If you are ok with ejecting you can disable it directly from webpack config I think

  25. Kleptine commented on Dec 25, 2018

    @Kleptine

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

  26. flyingcrp commented on Dec 29, 2018

    @flyingcrp

    oh...in CRA 2.1.1 how can i overwirte the esling rule .? i have own linting config. maybe i have ocd . :(

  27. locked and limited conversation to collaborators on Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions