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

[consistent-type-imports] Auto-fix not working well with ReactJS imports #2455

Closed
maneetgoyal opened this issue Sep 1, 2020 · 5 comments
Closed

Comments

@maneetgoyal
Copy link

@maneetgoyal maneetgoyal commented Sep 1, 2020

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

// eslintrc.json
{
  "rules": {
    "@typescript-eslint/consistent-type-imports": "error"
  }
}
// TS source file
import React from "react";

export const ComponentFoo: React.FC = () => {
  return <div>Foo Foo</div>;
};
// tsconfig.json

  "compilerOptions": {
    "sourceMap": true,
    "module": "es6",
    "moduleResolution": "node",
    "target": "es6",
    "jsx": "react",
    "declaration": true,
    "declarationMap": true,
    "isolatedModules": true,
    "strict": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "importHelpers": true,
    "esModuleInterop": true
  }

Expected Result

After auto-fix, I should be getting no change to my import line:

import React from "react";

export const ComponentFoo: React.FC = () => {
  return <div>Foo Foo</div>;
};

Actual Result

After auto-fix, getting:

import type React from "react";

export const SampleCollector: React.FC = () => {
  return <div>Sample Collector</div>;
};

And TSC is unhappy with it; saying:

'React' cannot be used as a value because it was imported using 'import type'.

Additional Info

Versions

package version
@typescript-eslint/eslint-plugin 4.0.1
@typescript-eslint/parser 4.0.1
TypeScript 4.0.2
ESLint 7.8.0
node 12.18.0
@doberkofler
Copy link
Contributor

@doberkofler doberkofler commented Sep 1, 2020

I have the same problem.

@bradzacher
Copy link
Member

@bradzacher bradzacher commented Sep 1, 2020

This is a really unfortunate side-effect of the react transform.
I hate that they made this a required part of it and I can't wait until they remove this requirement...

To properly support this, we'll have to add a configuration option to allow you to specify the JSX factory similar to tsconfig:

@bradzacher bradzacher added bug and removed triage labels Sep 1, 2020
@glen-84
Copy link
Contributor

@glen-84 glen-84 commented Sep 2, 2020

Is it not possible to read that from the TypeScript configuration?

@maneetgoyal
Copy link
Author

@maneetgoyal maneetgoyal commented Sep 2, 2020

That'll be so nice if it's doable, given we already provide a reference to tsconfig.json in eslintrc.json:

"parserOptions": {
    "parser": "@typescript-eslint/parser",
    "project": "./tsconfig.eslint.json",
    "ecmaFeatures": {
      "jsx": true
    }
  },
@bradzacher
Copy link
Member

@bradzacher bradzacher commented Sep 2, 2020

We can prefill it in that case, for sure!

However the rule does not require type information, and not everyone uses type-aware parsing. So we need to provide an option for those people that don't.

@bradzacher bradzacher self-assigned this Sep 6, 2020
bradzacher added a commit that referenced this issue Sep 6, 2020
Fixes #2455
And part of #2477

JSX is a first-class citizen of TS, so we should really support it as well.
I was going to just rely upon `eslint-plugin-react`'s patch lint rules (`react/jsx-uses-react` and `react/jsx-uses-vars`), but that leaves gaps in our tooling.
For example #2455, `consistent-type-imports` makes assumptions and can create invalid fixes for react without this change.
We could add options to that lint rule for the factory, but that is kind-of a sub-par experience and future rule authors will likely run into similar problems.

- Adds full scope analysis support for JSX.
- Adds two new `parserOption`:
    - `jsxPragma` - the name to use for constructing JSX elements. Defaults to `"React"`. Will be auto detected from the tsconfig.
    - `jsxFragmentName` - the name that unnamed JSX fragments use. Defaults to `null` (i.e. assumes `React.Fragment`). Will be auto detected from the tsconfig.
bradzacher added a commit that referenced this issue Sep 6, 2020
Fixes #2455
And part of #2477

JSX is a first-class citizen of TS, so we should really support it as well.
I was going to just rely upon `eslint-plugin-react`'s patch lint rules (`react/jsx-uses-react` and `react/jsx-uses-vars`), but that leaves gaps in our tooling.
For example #2455, `consistent-type-imports` makes assumptions and can create invalid fixes for react without this change.
We could add options to that lint rule for the factory, but that is kind-of a sub-par experience and future rule authors will likely run into similar problems.

- Adds full scope analysis support for JSX.
- Adds two new `parserOption`:
    - `jsxPragma` - the name to use for constructing JSX elements. Defaults to `"React"`. Will be auto detected from the tsconfig.
    - `jsxFragmentName` - the name that unnamed JSX fragments use. Defaults to `null` (i.e. assumes `React.Fragment`). Will be auto detected from the tsconfig.
phaux added a commit to phaux/typescript-eslint that referenced this issue Sep 28, 2020
…slint#2498)

Fixes typescript-eslint#2455
And part of typescript-eslint#2477

JSX is a first-class citizen of TS, so we should really support it as well.
I was going to just rely upon `eslint-plugin-react`'s patch lint rules (`react/jsx-uses-react` and `react/jsx-uses-vars`), but that leaves gaps in our tooling.
For example typescript-eslint#2455, `consistent-type-imports` makes assumptions and can create invalid fixes for react without this change.
We could add options to that lint rule for the factory, but that is kind-of a sub-par experience and future rule authors will likely run into similar problems.

- Adds full scope analysis support for JSX.
- Adds two new `parserOption`:
    - `jsxPragma` - the name to use for constructing JSX elements. Defaults to `"React"`. Will be auto detected from the tsconfig.
    - `jsxFragmentName` - the name that unnamed JSX fragments use. Defaults to `null` (i.e. assumes `React.Fragment`). Will be auto detected from the tsconfig.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants