Skip to content

New Typescript linting overriding user config and causing build failure #6871

Closed

Description

@ztolley

The introduction of #6513 stops my application from compiling as it uses it's own Typescript linting rules instead of the rules I have in .eslintrc.js

Previous to CRA adding Typescript linting I created my own typescript lint config for eslint using @typescript-eslint/eslint-plugin and @typescript-eslint/parser along with airbnb and prettier rules.

Within the project there is typescript file generated by graphql-code-generator that breaks various linting rules, so has been added to a .eslintignore file. When I run eslint the project passes with no errors.

However, when I build the project or run it in dev mode 2 lint errors are raised, relating to the fact that import statements appear half way through the file. Using the .eslintignore file doesn't fix this problem, nor does updating my .eslintrc.js file to exclude the rule.

As a result I cannot build my project with CRA 3 and have had to revert to 2.

Activity

  1. wilcoschoneveld commented on Apr 23, 2019

    @wilcoschoneveld

    I had the same issue but I found a solution: https://graphql-code-generator.com/docs/plugins/add

    In your codegen.yml, use the plugin add to insert an /* eslint-disable */ line to the start of the file.

    Example:

    schema: http://localhost:5000/api/graphql
    overwrite: true
    generates:
        ./src/generated.tsx:
            documents: ./src/graphql/*.ts
            plugins:
                - add: "/* eslint-disable */"
                - typescript-common
                - typescript-client
                - typescript-react-apollo
            config:
                noNamespaces: true
                enumsAsTypes: true
                withHooks: true
                noHoc: false
                noComponents: false
    
  2. jadbox commented on Apr 23, 2019

    @jadbox

    I'm having the same issue: tslint.json rules seem to be ignored now in v3 and overwritten by the new defaults. Please send help.

  3. proluk commented on Apr 24, 2019

    @proluk

    We've also upgraded to the latest react-scripts (3.0.0) in order to get rid of high vulnerabilities, but it broke our tslint configuration. No .eslinttignore or .eslintrc are working and adding comments to every file that disables eslint seems really bad.

  4. ianschmitz commented on Apr 24, 2019

    @ianschmitz
    Contributor

    Our ESLint configuration is not currently configurable (as has always been the case) and is documented here: https://facebook.github.io/create-react-app/docs/setting-up-your-editor#displaying-lint-output-in-the-editor. Any changes you make to your configuration file will only affect your editor integration but not the Dev server or prod builds.

    We haven't ever supported TSLint so I'm not sure how the other comments are related.

  5. raptor1989 commented on Apr 24, 2019

    @raptor1989

    Ok, but why can't I configure my own rules? To comply with your rules, I would have to rewrite my code. Stupid breaking change... I also came back to revert to 2.

  6. jadbox commented on Apr 24, 2019

    @jadbox

    @ianschmitz I think the confusion was that Typescript folks where using TSLint since the react-scripts had no TS linting support. We have entire projects built using custom rules, but now since TS linting is supported in v3, we are all stuck with pages of warnings and/or broken builds. While the JS community had somewhat the ease of greenfield development using react-scripts' blessed linting rules, the TS community is now coming onboard with custom opinionated rulesets for large existing projects.

    Proposals:

    1. provide a way to completely disable linting in production and live server (maybe this already exists?)
    2. provide a way for custom eslint overriding rules (both JS/TS)

    On a community aside, I didn't realize the react-scripts didn't allow JS projects to customize the linting rules. As someone that disagrees with the defaults of most linters (as well as my team), it feels very heavy-handed to including linting by-default without a way to customize. (obviously there's ejection, but it's far from an easy path to own)

  7. ztolley commented on Apr 24, 2019

    @ztolley
    Author

    I purposely use eslint as the Typescript team stated that that is their preferred solution and are putting effort into eslint support, though I'm sure some folks here use tslint.

    But back to the comment by @ianschmitz . I understand that the linting config, which now supports Typescript, is it's own thing and doesn't use the lint config a developer may add but my issue is that this recent change broken my code, and it seems others too. I know moving to 3 means possible breaking changes but this wasn't in the release notes, probably because no one realised.

    The workaround @wilcoschoneveld mentioned will get me out of this for now but as @jadbox says, linting is opinionated and projects have rules in place and probably went through their own pain getting them agreed. I too think that having CRA linting errors stop a build is too heavy handed. I worry that option 2 would be too difficult to do and maybe the quickest short term solution would be to add a config option to allow a build to complete even when there are lint errors and maybe another to allow linting to be disabled so the user can use their own.

  8. dannycochran commented on Apr 26, 2019

    @dannycochran

    @ianschmitz is this just a matter of looking to see if a user has provided an .eslintrc file here? if they have one, CRA could load it, otherwise use the existing one as a default?

    https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/config/webpack.config.js#L324

  9. ianschmitz commented on Apr 26, 2019

    @ianschmitz
    Contributor

    @ztolley The baked in linting configuration is how we've always provided CRA, including adding new rules between major versions which is similar to what is being mentioned in this thread. This thread is slightly different in that some (including myself) were using TSLint prior to 3.0. To clarify, we did mention that the added TypeScript linting was in the 3.0 release notes and listed as a breaking change.

    I will discuss the possibility of extending the ESLint config with the team which i think would satisfy what most of you are looking for (i know i would use it personally), but i can't promise anything as it's a fairly significant change to how we've been providing CRA for a few years now.

  10. self-assigned this
    on Apr 26, 2019
  11. drdxk commented on Apr 26, 2019

    @drdxk

    This is very frustrating.
    The fix seemingly solves very few, if any, actual problems people report, while going pretty much against what is purports to fix, i.e. #5641, which very clearly shows interest for a configurable tslint. In addition it enforces rules that are NOT in the list of "things that we want", such as unknown-ref,
    in a few examples that I looked, it seems to ignore things defined via definitely typed @types that have something like
    declare global {
    x: Type;
    }
    x is being reported as unkonwn.

  12. estaub commented on Apr 26, 2019

    @estaub

    @dmitryxcom Just to be clear, by "the fix", are you referring to 3.0, or to some subsequent PR?

  13. drdxk commented on Apr 26, 2019

    @drdxk

    @estaub I mean #6513

  14. 39 remaining items

  15. danielkcz commented on Jul 10, 2019

    @danielkcz

    Hm, I take it back, I thought that eslint src works, but it's actually false positive. It does not check any TS files. When I execute eslint --print-config src it uses babel-eslint and doesn't match against Typescript files.

    Running eslint --print-config src/index.tsx actually shows parser @typescript-eslint/parser used.

    I had to run the ugliness of eslint .\src\**\*.ts .\src\**\*.tsx to make it check correctly.

    This should be definitely documented. It's great we get lint checks with start script, but that's overkill if we want to just check files, especially in CI service.

  16. 0xb1ob commented on Jul 10, 2019

    @0xb1ob

    I'm using simply a glob pattern, ./src/**/*.{ts,tsx}
    plus maxwarning option equal to 0 to prevent commits when there are any warnings (by default warnings on CI are treated as errors).

  17. babakness commented on Jul 17, 2019

    @babakness

    Having the ability to define/override rules in eslintrc.json is pretty important...

  18. alexfoxgill commented on Jul 23, 2019

    @alexfoxgill

    I like to structure my TS code using namespaces to group functions, for example:

    export type Option<A> = Some<A> | None<A>
    
    export namespace Option {
      export const none = <A>(): Option<A> => new None<A>()
      export const some = <A>(a: A): Option<A> => new Some<A>(a)
    }
    
    ...

    CRA seems to want to tell me how to write my code:

    Compiling...
    Failed to compile.
    
    ./src/util/Option.ts
      Line 3:  ES2015 module syntax is preferred over custom TypeScript modules and namespaces  @typescript-eslint/no-namespace and namespaces  @typescript-eslint/no-namespace
    

    This is a purely stylistic point - giving a compilation error! - and actually one that makes zero sense, since namespaces are actually useful.

    How do I get rid of this useless error?

    (the workaround, for those others frustrated with this limitation, is to declare a const object with a property for each namespace member)

  19. danielkcz commented on Jul 23, 2019

    @danielkcz

    AFAIK namespaces are still not supported.

    Edit: Ah, so there is some experimental support ... https://babeljs.io/docs/en/babel-plugin-transform-typescript/

  20. talaikis commented on Aug 11, 2019

    @talaikis

    This one proves the rule that software updates are the scams to sell further fixes for those updates.

  21. loganknecht commented on Aug 11, 2019

    @loganknecht

    Hi. I'm just chiming in here because I got side-swiped by having to upgrade create-react-app from version 2.1 to 3.0 this weekend, for a project that is fairly mature, in order to fix my build times for my react app.

    Please correct me if I'm wrong, but my understanding is:

    • my upgrade moves me away from react-scripts-ts to react-scripts
    • I can have an .eslintrc.json file to help my IDE/local development environment
    • I can't disable react build script warnings because the team thinks that's inappropriate, and react-scripts provides no ability to do so
    • Which means all of my builds will have failures because my development settings don't align with the build settings
    • There is no clear work around

    If this is is the case, I am beyond disappointed in what the perceived user experience is for this.

    A developer is always going to want to toggle these settings. All of these settings may or may not directly apply to specific projects. And now the product create-react-app is blasting noise every time I build which makes it impossible to discern what is a failure, and what is actually something I need to address. Which means that pushes all of that output into my IDE. And even worse, is these are displayed as warnings in a lot of instances when they should be failures blocking me until I fix them.

    Folks, this is terrible, and I would like to articulate that this lack of support is a big disappointment that makes me very keen to move away from create-react-app as it basically removes a significant amount of functionality that made it attractive in the first place.

  22. MattMorrisDev commented on Aug 11, 2019

    @MattMorrisDev

    If you want to totally disable the build warning overlays, you can add this css to your project as a short term / quick fix:

    iframe[style="position: fixed; top: 0px; left: 0px; width: 100%; height: 100%; border: none; z-index: 2147483647;"] {
            display: none;
    }
    
  23. bugzpodder commented on Aug 11, 2019

    @bugzpodder

    3.1 comes with support for custom eslint configuration, should that solve most of the problems here?

    #7036

  24. StJohn3D commented on Aug 21, 2019

    @StJohn3D

    @bugzpodder Over an hour later and I haven't had any luck getting that to work. Am I missing something?

    .env

    EXTEND_ESLINT=true
    

    package.json (The relevant parts)

    {
      "dependencies": {
         "react-scripts": "3.1.1"
      },
     
      "eslintConfig": {
        "extends": [
          "react-app",
          "eslint-config-react-app"
        ],
        "rules": {
          "no-useless-escape": "off"
        },
        "overrides": [
          {
            "files": [
              "**/*.ts?(x)"
            ],
            "rules": {
              "no-useless-escape": "off"
            }
          }
        ]
      }
    }
  25. djkirby commented on Aug 22, 2019

    @djkirby

    @StJohn3D I would double check that there aren't any errors being swallowed -- #7530.

    Might wanna check by adding this line in to your node_modules/react-scripts/config/webpack.config.js https://github.com/facebook/create-react-app/pull/7530/files#diff-dc0c4e7c623b73660da1809fc60cf6baR349

  26. ianschmitz commented on Oct 16, 2019

    @ianschmitz
    Contributor

    @ztolley have you tried enabling extending our built-in ESLint config as described here: https://create-react-app.dev/docs/setting-up-your-editor/#experimental-extending-the-eslint-config?

    Let us know so I can close this if it solves your issues.

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions