gajus / eslint-plugin-jsdoc

JSDoc specific linting rules for ESLint.
Other
1.1k stars 159 forks source link

Add a TypeScript config for mixed TS/JS projects #1101

Open jaydenseric opened 1 year ago

jaydenseric commented 1 year ago

Motivation

At the moment, there isn’t a config for a TypeScript project that has type checking in the TypeScript modules (.ts, .tsx, .mts, .cts) as well as the JavaScript modules (.js, .mjs, .cjs). Quite often you will have a src/ dir that contains all the .mts files for TypeScript to compile to dist/, but you also have files like .eslintrc.js that don't contain TypeScript syntax but still are type checked by TypeScript via the // @ts-check comment and TypeScript flavour of JSDoc comments.

It would be good to add a eslint-plugin-jsdoc recommended config for TypeScript mixed TS/JS projects, deprecating the current recommended TypeScript configs that assume a TypeScript type checked project is only TS, or only JS, but not mixed.

Current behavior

To try to get things working with v44, here was my approach in .eslintrc.js (with all non eslint-plugin-jsdoc config removed for brevity):

// @ts-check

/** @type {import("eslint").Linter.Config} */
module.exports = {
  root: true,
  extends: [
    "plugin:jsdoc/recommended-typescript-error",
  ],
  rules: {
    // Only TypeScript should check JSDoc types, see:
    // https://github.com/gajus/eslint-plugin-jsdoc/issues/888#issuecomment-1544914446
    "jsdoc/no-undefined-types": "off",
  },
  overrides: [
    {
      // ESM and CJS JavaScript modules.
      files: ["*.mjs", "*.cjs", "*.js"],
      rules: {
        "jsdoc/check-tag-names": [
          "error",
          {
            // Allow type related JSDoc tags like `@type`, as TypeScript syntax
            // is not available in these JavaScript modules.
            typed: false,
          },
        ],
        // Allow types in JSDoc tags like `@param`, as TypeScript syntax is not
        // available in these JavaScript modules.
        "jsdoc/no-types": "off",
      },
    },
  ],
};

This approach of picking a config and then trying to overwrite it with overrides is frustrating to figure out and maintain, and most people don't understand how all this fits together well enough to get it right.

Desired behavior

There would be a recommended config that targets the specific types of JavaScript and TypeScript modules that can exist in a project by file extension, applying only the relevant JSDoc rules to each kind of module.

Alternatives considered

brettz9 commented 1 year ago

I like the idea, but I might note that the job has already been made considerably easier by letting the typescript configs--including a new one for typescript-flavor which works on plain JavaScript and does not exclude types--exclude no-undefined-types. Note that the new recommended typescript flavor config may be useful for those who enable JavaScript via allowJs (and possibly checkJs) in tsconfig.json, not requiring any // @ts-check comment.

I'm in favor of a config which handles the overrides and such for people, though I think we should keep the more atomic configs, especially as projects might have exceptions in whether they are type-checking all JavaScript or not (and ts-check-detection might not be the criterion if, again, their JavaScript enabling is done through tsconfig.json).

But I do think it is useful to have one config to handle it for people.

geoffswift commented 10 months ago

We had problems with our JavaScript only code base flagging up warnings like so...

Use object shorthand or index signatures instead of `Object`, e.g., `{[key: string]: string}`  jsdoc/check-types

Using the TypeScript recommended fix solved the warning, but JSDoc itself treats it as a syntax error. The solution ultimately was to change the setting jsdoc.mode for esling-plugin-jsdoc as follows (since the default is 'typescript').

    settings: {
        jsdoc: {
            mode: 'jsdoc'
        }
    }

It took a while to delve through the documentation to figure this out, and I was left thinking:

The recommendation from jaydenseric to apply rules appropriately based on file extension sounds ideal from my perspective.

brettz9 commented 10 months ago

We had problems with our JavaScript only code base flagging up warnings like so...

Use object shorthand or index signatures instead of `Object`, e.g., `{[key: string]: string}`  jsdoc/check-types

Using the TypeScript recommended fix solved the warning, but JSDoc itself treats it as a syntax error. The solution ultimately was to change the setting jsdoc.mode for esling-plugin-jsdoc as follows (since the default is 'typescript').

  settings: {
      jsdoc: {
          mode: 'jsdoc'
      }
  }

It took a while to delve through the documentation to figure this out, and I was left thinking:

  • why should it be treating a file with the extension*.js as TypeScript ?

It's not treating the file as TypeScript; it's treating the JSDoc as being of the TypeScript flavor of JSDoc. It can still work in JavaScript.

  • if I later wanted to introduce TypeScript into the code base, how should I configure this setting?

You should get rid of the mode addition you added. You can technically use Object with mode: "typescript" (by modifying preferredTypes), but it is discouraged by default. See check-types for a fuller discussion.

The recommendation from jaydenseric to apply rules appropriately based on file extension sounds ideal from my perspective.

The recommendation is a good one, but it won't apply to your situation.

Also, as an update, given that flat config has dropped overrides, we'll have to look to use a different mechanism for any such config. I'd prefer to wait for the dust to settle as ESLint 9 (which uses flat config now by default) is only in alpha.