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

Rule proposal: warn against import declarations with no value specifiers and only inline type specifiers #6382

Closed
6 tasks done
bradzacher opened this issue Jan 27, 2023 · 2 comments · Fixed by #6394
Closed
6 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@bradzacher
Copy link
Member

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • My proposal specifically checks TypeScript syntax, or it proposes a check that requires type information to be accurate.
  • My proposal is not a "formatting rule"; meaning it does not just enforce how code is formatted (whitespace, brace placement, etc).
  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

Spinning off of import-js/eslint-plugin-import#2676

The new verbatimModuleSyntax flag will not elide an import declaration if the only specifiers are inline type specifiers:

import {type A, type B} from 'mod';

// transpiled to

import {} from 'mod';
// which is the same as
import 'mod';

This is obviously a problem - code that looks like it is pure type-only code can have runtime side-effects!

After some discussion, we've come to a decision that there needs to exist a rule that warns against this code. The behaviour doesn't really fit into any existing lint rule, even as an option. Additionaly as it's so hyper-targeted on TS behaviour, it doesn't make sense to add this rule to eslint-plugin-import.

Thus we should add the rule to our plugin.

At the same time I think we should add documentation about how to use the lint rules together. Copying from the other issue:

  1. add a new lint rule which bans imports that have no value specifiers, and only type specifiers
  2. anyone who wishes to use inline type specifiers by default and NOT use the TS flag, use import/consistent-type-specifier-style together with import/no-duplicates with the {"prefer-inline": true} config.
  3. anyone who wishes to use inline type specifiers by default AND use the TS flag, just use import/no-duplicates with the {"prefer-inline": true} config in conjunction with the new rule from (1)
  4. anyone who wishes to use top-level type specifiers by default (with or without the TS flag), use import/consistent-type-specifier-style together with import/no-duplicates with the {"prefer-inline": false} config.

Fail Cases

import {type A, type B} from 'mod';

Pass Cases

import 'mod';
import * as mod from 'mod';
import type * as mod from 'mod';
import Foo from 'mod';
import type Foo from 'mod';
import {Bar} from 'mod';
import type {Bar} from 'mod';
import Foo, {Bar} from 'mod';
import Foo, {type Bar} from 'mod';
import {Bar, Baz} from 'mod';
import {type Bar, Baz} from 'mod';

Additional Info

#6380

@andrewbranch
Copy link

Note that this would have to be combined with consistent-type-imports in order to replace importsNotUsedAsValues, because code like

import { SomeClass } from "./mod";

function foo(x: SomeClass) {}

is legal in verbatimModuleSyntax as long as SomeClass has a real value. I think that’s fine, just wanted to point it out.

@bradzacher
Copy link
Member Author

Oh that's really interesting. I didn't realise that the flag wouldn't enforce that is marked as a type - I just assumed it would.

The only issue I see with using consistent-type-imports and verbatimModuleSyntax is that you'll get double errors around the place, which a lot of people hate. But other than that yeah the rule will strictly enforce (with an autofixer!) all cases.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
2 participants