Skip to content
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

[Bug]: TS types are broken #3838

Closed
2 tasks done
burtek opened this issue Oct 1, 2024 · 16 comments · Fixed by #3840
Closed
2 tasks done

[Bug]: TS types are broken #3838

burtek opened this issue Oct 1, 2024 · 16 comments · Fixed by #3840

Comments

@burtek
Copy link
Contributor

burtek commented Oct 1, 2024

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

Description Overview

The types as exported in #3797 and #3836 are broken and throw TS errors while used. The package only exports named namespace of configs, which means you can't do any of the following:

import react from 'eslint-plugin-react';
import { config } from 'typescript-eslint';

export default config({
    plugin: { react },
    rules: {
        ...react.configs.recommended.rules,
        ...someMoreRules
    }
});

image

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBFApgQwMbwGZQiOByRAZwBtgA7GAWjGIFcBzcypNGPAbgChRJY4BvOKghkMwenAC+cLDnwwAnmCKoowMFSKkKHTp2FlC8QjkQBZaIgBKtYkTgBeAZK6dEAD17wAJogzJbeAMxegAKfk44KLgaBnJCAC4BBBR0KQAaSOioWyIkiOjCuAA6UpZ0YuDxQmKkYRAQRDJfb1rcwkyi6NLik0aLJBs7QiyoyU5JAEp2IA

Expected Behavior

No TS error.

eslint-plugin-react version

v7.37.1

eslint version

v8.57.0

node version

v20.9.0

typescript version

All, tested on v5.5.2, v.5.6.2

@JstnMcBrd
Copy link

This appears to be because only the flat configs are included in the types. You can do reactPlugin.configs.flat.recommended, but you can't access the legacy configs with reactPlugin.configs.recommended.

@burtek
Copy link
Contributor Author

burtek commented Oct 1, 2024

Oh, that's true (though still not in line with what's actually exported). But I still can't do plugin: { react }

@burtek
Copy link
Contributor Author

burtek commented Oct 1, 2024

And the flat config is not typescript-eslint compatible either...

image

@JstnMcBrd
Copy link

Yes, I was able to add that - even the flat config types are broken. They appear to be incompatible with ESLint's types.

import type { Linter } from 'eslint';

export function reactConfig(): Linter.FlatConfig[] {
  return [
    reactPlugin.configs.flat.recommended,
    reactPlugin.configs.flat['jsx-runtime'],
  ];
}
Type '{ plugins: { react: { deprecatedRules: any; rules: { 'boolean-prop-naming': RuleModule; 'button-has-type': RuleModule; 'checked-requires-onchange-or-readonly': RuleModule; ... 99 more ...; 'void-dom-elements-no-children': RuleModule; }; configs: { ...; }; }; }; rules: { ...; }; languageOptions: { ...; }; }' is not assignable to type 'FlatConfig<RulesRecord>'.
  Types of property 'plugins' are incompatible.
    Type '{ react: { deprecatedRules: any; rules: { 'boolean-prop-naming': RuleModule; 'button-has-type': RuleModule; 'checked-requires-onchange-or-readonly': RuleModule; ... 99 more ...; 'void-dom-elements-no-children': RuleModule; }; configs: { ...; }; }; }' is not assignable to type 'Record<string, Plugin>'.
      Property 'react' is incompatible with index signature.
        Type '{ deprecatedRules: any; rules: { 'boolean-prop-naming': RuleModule; 'button-has-type': RuleModule; 'checked-requires-onchange-or-readonly': RuleModule; ... 99 more ...; 'void-dom-elements-no-children': RuleModule; }; configs: { ...; }; }' is not assignable to type 'Plugin'.
          Types of property 'configs' are incompatible.
            Type '{ recommended: { plugins: string[]; parserOptions: { ecmaFeatures: { jsx: boolean; }; }; rules: { 'react/display-name': number; 'react/jsx-key': number; 'react/jsx-no-comment-textnodes': number; 'react/jsx-no-duplicate-props': number; ... 17 more ...; 'react/require-render-return': number; }; }; all: { ...; }; 'jsx-...' is not assignable to type 'Record<string, FlatConfig<RulesRecord> | FlatConfig<RulesRecord>[] | ConfigData<RulesRecord>>'.
              Property 'recommended' is incompatible with index signature.
                Type '{ plugins: string[]; parserOptions: { ecmaFeatures: { jsx: boolean; }; }; rules: { 'react/display-name': number; 'react/jsx-key': number; 'react/jsx-no-comment-textnodes': number; 'react/jsx-no-duplicate-props': number; ... 17 more ...; 'react/require-render-return': number; }; }' is not assignable to type 'FlatConfig<RulesRecord> | FlatConfig<RulesRecord>[] | ConfigData<RulesRecord>'.
                  Type '{ plugins: string[]; parserOptions: { ecmaFeatures: { jsx: boolean; }; }; rules: { 'react/display-name': number; 'react/jsx-key': number; 'react/jsx-no-comment-textnodes': number; 'react/jsx-no-duplicate-props': number; ... 17 more ...; 'react/require-render-return': number; }; }' is not assignable to type 'ConfigData<RulesRecord>'.
                    Types of property 'rules' are incompatible.
                      Type '{ 'react/display-name': number; 'react/jsx-key': number; 'react/jsx-no-comment-textnodes': number; 'react/jsx-no-duplicate-props': number; 'react/jsx-no-target-blank': number; 'react/jsx-no-undef': number; ... 15 more ...; 'react/require-render-return': number; }' is not assignable to type 'Partial<RulesRecord>'.
                        Property ''react/display-name'' is incompatible with index signature.
                          Type 'number' is not assignable to type 'RuleEntry<any[]> | undefined'.ts(2322)

On first glance, I wonder if this is because the type appears to have some recursion. It's possible to do reactPlugin.configs.flat.recommended.plugins.react.configs.recommended.

image

The flat configs contain the legacy configs. Is that intentional? That might be the problem.

@ljharb
Copy link
Member

ljharb commented Oct 2, 2024

@JstnMcBrd yes, that's intentional and unavoidable with the current architecture of the plugin.

@burtek
Copy link
Contributor Author

burtek commented Oct 2, 2024

I can probably look into fixing types today or tomorrow, no promises though

EDIT: so far wasn't able to

@damisparks
Copy link

I am having the same issue.

      react: eslintPluginReact,
Type '{ deprecatedRules: any; rules: { 'boolean-prop-naming': Rule.RuleModule; 'button-has-type': Rule.RuleModule; 'checked-requires-onchange-or-readonly': Rule.RuleModule; ... 99 more ...; 'void-dom-elements-no-children': Rule.RuleModule; }; configs: { ...; }; }' is not assignable to type 'Omit<Plugin, "configs">'.
  Types of property 'rules' are incompatible.
    Type '{ 'boolean-prop-naming': Rule.RuleModule; 'button-has-type': Rule.RuleModule; 'checked-requires-onchange-or-readonly': Rule.RuleModule; 'default-props-match-prop-types': Rule.RuleModule; ... 98 more ...; 'void-dom-elements-no-children': Rule.RuleModule; }' is not assignable to type 'Record<string, LooseRuleDefinition>'.
      Property ''jsx-no-literals'' is incompatible with index signature.
        Type '{ meta: RuleModule["meta"]; create(context: any): (false | { ImportDeclaration(node: any): void; VariableDeclaration(node: any): void; }) & { ...; }; }' is not assignable to type 'LooseRuleDefinition'.
          Type '{ meta: RuleModule["meta"]; create(context: any): (false | { ImportDeclaration(node: any): void; VariableDeclaration(node: any): void; }) & { ...; }; }' is not assignable to type '{ create: LooseRuleCreateFunction; meta?: object | undefined; }'.
            The types returned by 'create(...)' are incompatible between these types.
              Type '(false | { ImportDeclaration(node: any): void; VariableDeclaration(node: any): void; }) & { Literal(node: any): void; JSXAttribute(node: any): void; JSXText(node: any): void; TemplateLiteral(node: any): void; }' is not assignable to type 'Record<string, Function | undefined>'.
                Type 'false & { Literal(node: any): void; JSXAttribute(node: any): void; JSXText(node: any): void; TemplateLiteral(node: any): void; }' is not assignable to type 'Record<string, Function | undefined>'.
                  Index signature for type 'string' is missing in type 'Boolean & { Literal(node: any): void; JSXAttribute(node: any): void; JSXText(node: any): void; TemplateLiteral(node: any): void; }'

@karlhorky
Copy link
Contributor

karlhorky commented Oct 6, 2024

Workaround

Use JSDoc-style type assertion to fix the types:

eslint.config.js

import react from 'eslint-plugin-react';

export const config = [
  {
    plugins: {
      // Type assertion is workaround for incorrect TypeScript
      // types in eslint-plugin-react
      //
      // TODO: Remove when types are fixed in eslint-plugin-react
      // - https://github.com/jsx-eslint/eslint-plugin-react/issues/3838
      react: /** @type {import('eslint').ESLint.Plugin} */ (react),

Or, if you use eslint.config.ts (ESLint 9.9.0+ experimental feature), use a TypeScript type assertion:

eslint.config.ts

import type { ESLint } from 'eslint';
import react from 'eslint-plugin-react';

export const config = [
  {
    plugins: {
      // Type assertion is workaround for incorrect TypeScript
      // types in eslint-plugin-react
      //
      // TODO: Remove when types are fixed in eslint-plugin-react
      // - https://github.com/jsx-eslint/eslint-plugin-react/issues/3838
      react: react as ESLint.Plugin,

@karlhorky
Copy link
Contributor

@ocavue @ljharb thanks for the PR, review and merge in #3840 🙌

I guess this will be released in a new patch or minor version?

@ljharb ljharb marked this as a duplicate of #3867 Dec 12, 2024
@carlos-lopez-SaRa

This comment has been minimized.

@ljharb

This comment has been minimized.

@carlos-lopez-SaRa

This comment has been minimized.

@ljharb

This comment has been minimized.

@karlhorky

This comment has been minimized.

@ljharb

This comment has been minimized.

@carlos-lopez-SaRa
Copy link

carlos-lopez-SaRa commented Dec 16, 2024

@carlos-lopez-SaRa that's not how github works; every project closes issues when it's merged, not when it's released.

The reason it's closed on merge is that the PR noted in the description it closes this issue, it is how Github works, whether should or not, we have little control over it.

Most popular & well maintained open source projects I've seen will reduce friction by either providing an immediate deployment, or a timeline for it, not saying this repo should do it, just noting my own experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

6 participants