Code Monkey home page Code Monkey logo

Comments (12)

OlivierZal avatar OlivierZal commented on June 11, 2024 2

Hi @bradzacher, I refined the description.

from typescript-eslint.

bradzacher avatar bradzacher commented on June 11, 2024 2

I would just disable all typescript-eslint rules.

I don't think that's a good summary, no.
Many of our rules work just fine in JS files!
Type-aware rules work just fine in JS files - so long as you're declaring half-decent JSDoc types they'll all work as expected. It's just that JS files are often lazily typed (if at all) and the rules do essentially function on "garbage in, garbage out".

IIRC there's only three rules which will explicitly not work in a JS file:

  • explicit-function-return-type
  • explicit-module-boundary-types
  • parameter-properties

Everything else will work just fine (and the TS-specific stuff just never matches).

There could be some value in having a config to disable the above 3 rules as in "disable-requiring-type-syntax" or something? That way you could declare like

export default tseslint.config({
  files: ["**/*.js"],
  extends: [tseslint.configs.disableRequiringTypeSyntax, tseslint.configs.disableTypeChecked],
});

from typescript-eslint.

bradzacher avatar bradzacher commented on June 11, 2024 1

The issue is that the way the docs are currently written, is that it makes it seem like disable-type-checked is all that you need to do to get the setup working with js files

If you're using the recommended config - then disable-type-checked is all you need 🙂

It's only when people branch out into custom configs that they need more!
For those custom usecases we do have it documented to some degree (docs that likely need updating 😅):

from typescript-eslint.

OlivierZal avatar OlivierZal commented on June 11, 2024

Closing this issue after @bradzacher closed the related PR.

from typescript-eslint.

bradzacher avatar bradzacher commented on June 11, 2024

@OlivierZal we can still discuss this idea here! We just like to start with an issue so we don't put the cart before the horse and have you burn time on work that might not be accepted.

from typescript-eslint.

kirkwaiblinger avatar kirkwaiblinger commented on June 11, 2024

TLDR - I don't think you want to use disable-type-checked for disabling typescript-eslint rules in *.js files. I would just disable all typescript-eslint rules.


I guess it's a question of what the purpose of the disable-type-checked config is. The example given in the docs shows it as being used to disable type-checked rules for **/*.js files.

To me, that seems wrong. Granted our "New Rule" issue template says "My proposal specifically checks TypeScript syntax, or it proposes a check that requires type information to be accurate.", if a rule

  1. deals with types, it might partially work in a .js file (e.g. no-floating-promises works).
  2. deals with TS syntax, it definitely should not be expected to work in a .js file. (see also parameter-properties false positive)

So, there should be no scenario where you'd only want typed lint rules disabled from JS files, right?

So, I come back to - that seems like a wrong use of the disable-type-checked config. I'd think instead, you'd simply... not want to enable typescript-eslint rules in the first place in a JS file. And disable-type-checked would be used for TS files you might have in your repo that you can't/don't want to set up with typed linting, or for removing type-aware rules out of a config that you've imported from somewhere else.

@bradzacher does that align with your understanding?

from typescript-eslint.

kirkwaiblinger avatar kirkwaiblinger commented on June 11, 2024

I don't think that's a good summary, no.

Lol, savage 😅 But, after reading your response, I think you're right 😆

Many of our rules work just fine in JS files! Type-aware rules work just fine in JS files - so long as you're declaring half-decent JSDoc types they'll all work as expected. It's just that JS files are often lazily typed (if at all) and the rules do essentially function on "garbage in, garbage out".

This is actually very cool! I haven't used the jsdoc-typed-js workflow yet, so I learned something new today.

I think this does prove the point that using disable-type-checked is not useful where what you would want is disable-rules-that-wont-work-in-js-files.

IIRC there's only three rules which will explicitly not work in a JS file:

  • explicit-function-return-type
  • explicit-module-boundary-types
  • parameter-properties

Looking around a bit, typedef should probably go in there too, since that's all about adding type annotations. Didn't find any other obvious examples.

There could be some value in having a config to disable the above 3 rules as in "disable-requiring-type-syntax" or something? That way you could declare like

export default tseslint.config({
  files: ["**/*.js"],
  extends: [tseslint.configs.disableRequiringTypeSyntax, tseslint.configs.disableTypeChecked],
});

+1


Can we take an action item to change the docs to not give **/*.js as the example for when to use disable-type-checked? Granted this discussion, that feels like a total red herring.

And we can split out the idea of disableRequiringTypeSyntax/disableRulesThatWontWorkInJSFiles to a separate issue?

from typescript-eslint.

bradzacher avatar bradzacher commented on June 11, 2024

Can we take an action item to change the docs to not give **/*.js as the example for when to use disable-type-checked?

The reason we use this as our example is because in modern TS codebases that's what most users want to do!

Having .js files is rare in a modern TS codebase and they're usually only there because you're using a tool that doesn't support TS (eg nodejs entrypoints or tooling config files).

The important bit is that because of their nature these files are rarely distributed to users and so people don't bother trying to create a tsconfig setup that matches those files.
Which is the difficult thing about type-aware linting - if you don't cover those files with a tsconfig then you get eslint failures on those files.

So the common user story as a modern TS codebase is "I have JS files for config but I don't want to try and make them work with type-aware linting so I want to disable type-aware linting altogether".

But ofc there are some codebases that are all-in on JS with JSDoc plus TS and type-aware linting. So in those codebases the above story does not apply. These codebases are the very rare case though cos jsdoc typing is cumbersome AF.

Its worth noting that with the new project service we've added better support for this usecase as these files can be automatically covered by a "default program". But it's still likely that people don't want type-aware rules because JSDoc typing is cumbersome AF and often times you haven't bothered grabbing @types packages for the dev deps - or there aren't even types for the packages (eg see the current state of eslint plugins and flat config files)

from typescript-eslint.

OlivierZal avatar OlivierZal commented on June 11, 2024

@bradzacher,

Having .js files is rare in a modern TS codebase and they're usually only there because you're using a tool that doesn't support TS (eg nodejs entrypoints or tooling config files).

I totally agree: I have only TS files... except the eslint flat config, which as far as I know must be a file named exactly eslint.config.js (kind of irony, isn't it?), and that's in this file I want to disable useless type-related eslint errors, although I want them elsewhere.

from typescript-eslint.

kirkwaiblinger avatar kirkwaiblinger commented on June 11, 2024

And, fair enough, but wouldn't it more helpful to the user to achieve that goal having a doc section that looks (very very) roughly like


Working with a mixed JS/TS codebase

Many of our rules actually work just fine in JS files!

However, some of our rules will cause spurious errors if they're applied to JS files, so you'll need to disable them.

export default tseslint.config(
  eslint.configs.recommended,
  ...tseslint.configs.recommendedTypeChecked,
  {
    languageOptions: {
      parserOptions: {
        project: true,
        tsconfigDirName: import.meta.dirname,
      },
    },
  },
+  {
+    // These rules will try to enforce TS syntax which is illegal inside a JS file
+    // maybe one day this will be included in a utility config rather than listed out individually
+    files: ['**/*.js'],
+    rules: [
+         "@typescript-eslint/explicit-function-return-types": "off",
+         "@typescript-eslint/parameter-properties": "off",
+         "@typescript-eslint/explicit-module-boundary-types": "off",
+         "@typescript-eslint/typedef": "off",
+    ]
+  },
);

Depending on your setup, our type-aware rules may cause problems as well. We provide the utility disable-type-checked config to help you disable them too

export default tseslint.config(
  eslint.configs.recommended,
  ...tseslint.configs.recommendedTypeChecked,
  {
    languageOptions: {
      parserOptions: {
        project: true,
        tsconfigDirName: import.meta.dirname,
      },
    },
  },
  {
    // These rules will try to enforce TS syntax which is illegal inside a JS file
    files: ['**/*.js'],
    rules: [
         "@typescript-eslint/explicit-function-return-types": "off",
         "@typescript-eslint/parameter-properties": "off",
         "@typescript-eslint/explicit-module-boundary-types": "off",
         "@typescript-eslint/typedef": "off",
    ]
  },
+  {
+    files: ['**/*.js'],
+    ...tseslint.configs.disableTypeChecked,
+  }
);

Alternately, if you use JS doc types in your JS files (helpful-link-to-more-info-on-this-for-people-who-don't-know-what-that-is), our type-aware rules will work, as long as the corresponding tsconfig is included in the eslint config.


The issue is that the way the docs are currently written, is that it makes it seem like disable-type-checked is all that you need to do to get the setup working with js files, which is how we got to this issue report in the first place.

from typescript-eslint.

kirkwaiblinger avatar kirkwaiblinger commented on June 11, 2024

also, lol on the eslint.config.js being the culprit 😆

from typescript-eslint.

Josh-Cena avatar Josh-Cena commented on June 11, 2024

Should we have a config like no-typescript-syntax-config that turns off all rules that add or modify TypeScript syntax...?

from typescript-eslint.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.