Skip to content

Commit

Permalink
feat(eslint-plugin): [consistent-type-imports] ignore files with deco…
Browse files Browse the repository at this point in the history
…rators, experimentalDecorators, and emitDecoratorMetadata
  • Loading branch information
bradzacher committed Feb 1, 2024
1 parent 8de6ee2 commit f3525d4
Show file tree
Hide file tree
Showing 34 changed files with 2,282 additions and 5,031 deletions.
7 changes: 7 additions & 0 deletions docs/packages/Parser.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ interface ParserOptions {
};
ecmaVersion?: number | 'latest';
emitDecoratorMetadata?: boolean;
experimentalDecorators?: boolean;
extraFileExtensions?: string[];
jsDocParsingMode?: 'all' | 'none' | 'type-info';
jsxFragmentName?: string | null;
Expand Down Expand Up @@ -128,6 +129,12 @@ Specifies the version of ECMAScript syntax you want to use. This is used by the
This option allow you to tell parser to act as if `emitDecoratorMetadata: true` is set in `tsconfig.json`, but without [type-aware linting](../linting/Typed_Linting.mdx). In other words, you don't have to specify `parserOptions.project` in this case, making the linting process faster.

### `experimentalDecorators`

> Default `undefined`.
This option allow you to tell parser to act as if `experimentalDecorators: true` is set in `tsconfig.json`, but without [type-aware linting](../linting/Typed_Linting.mdx). In other words, you don't have to specify `parserOptions.project` in this case, making the linting process faster.

### `extraFileExtensions`

> Default `undefined`.
Expand Down
65 changes: 62 additions & 3 deletions packages/eslint-plugin/docs/rules/consistent-type-imports.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,70 @@ type T = import('Foo').Foo;
const x: import('Bar') = 1;
```

## Usage with `emitDecoratorMetadata`
## Caveat: `@decorators` + `experimentalDecorators: true` + `emitDecoratorMetadata: true`

The `emitDecoratorMetadata` compiler option changes the code the TypeScript emits. In short - it causes TypeScript to create references to value imports when they are used in a type-only location. If you are using `emitDecoratorMetadata` then our tooling will require additional information in order for the rule to work correctly.
> TL;DR - the rule will **_not_** report any errors in files _that contain decorators_ when **both** `experimentalDecorators` and `emitDecoratorMetadata` are turned on.
If you are using [type-aware linting](https://typescript-eslint.io/linting/typed-linting), then you just need to ensure that the `tsconfig.json` you've configured for `parserOptions.project` has `emitDecoratorMetadata` turned on. Otherwise you can explicitly tell our tooling to analyze your code as if the compiler option was turned on [by setting `parserOptions.emitDecoratorMetadata` to `true`](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/README.md#parseroptionsemitdecoratormetadata).
When both the compiler options `experimentalDecorators` and `emitDecoratorMetadata` are turned on and a class is annotated with decorators, TypeScript will emit runtime metadata for the class that captures the property types, method parameter types, and method return types. This runtime code is derived using type information - meaning that the runtime code emitted changes based on the cross-file type information that TypeScript has computed.

This behavior is problematic for the style enforced by this lint rule because it also means that annotating an import with `type` can change the runtime metadata! For example in the following snippet TS:

```ts
import Foo from 'foo';
import decorator from 'decorator';

class Clazz {
@decorator
method(arg: Foo) {}
}
```

TS will emit metadata for `method`'s `arg` to annotate its type:

- If `Foo` is a value (eg a class declaration), then TS will annotate the type of `arg` as `Foo`
- Put another way - there is an invisible runtime reference to `Foo`!
- If `Foo` is a type (eg an interface), then TS will annotate the type of `arg` as either `Object`, `String`, etc - depending on what the type is.

Syntactically it _looks_ like the `Foo` import is a type-only import because it's only used in a type location. However if we annotate the `Foo` import as `type` then it means that the runtime code always falls into the latter case of a type - which changes the runtime metadata! This is a problem for the rule because the rule is not type-aware - so it can't determine if an import is a type or a value - it works purely based on how the import is used.

In the past we tried to solve this problem by enforcing that imported names that are used in decorator metadata are specifically _not_ marked as type-only imports to ensure that values are correctly emitted in the runtime code.

However things get further complicated if you also turn on `isolatedModules` because TS will start enforcing that your imported types are marked as type-only. So to satisfy this the rule needs to enforce that imported values used in decorator metadata are imported _without_ a `type` qualifier, and imported types that are used in decorator metadata are imported _with_ a `type` qualifier. So our above solution of always removing the `type` doesn't work, and instead we need type-information to correctly report errors and fix code.

Ultimately we decided to just opt-out of handling this use-case entirely to avoid accidentally reporting the wrong thing and fixing to code that either fails to compile or alters the emitted runtime metadata. If you'd like more information please [check out the related issue and its discussion](https://github.com/typescript-eslint/typescript-eslint/issues/5468).

To be clear this means the following - the rule will **_not_** report any errors in files _that contain decorators_ when **both** `experimentalDecorators` and `emitDecoratorMetadata` are turned on.

If you are working in such a workspace and want to correctly have your imports consistently marked with `type`, we suggest using the [`verbatimModuleSyntax`](https://www.typescriptlang.org/tsconfig#verbatimModuleSyntax) compiler option which will use type information to correctly enforce that types are marked with `type` and values are not when they are used in decorator metadata.

### Note: TypeScript v5.0 (+ v5.2) decorators

[TypeScript v5.0](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-0.html#decorators) added support for the latest stable decorator proposal. [TypeScript v5.2](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-2.html#decorator-metadata) added support for the latest stable decorator metadata proposal. If you are writing decorators on TS v5.2+ with `emitDecoratorMetadata: true` and `experimentalDecorators: false` then you are using the new decorator metadata support.

The new decorator metadata does not include any annotations derived from types -- meaning it does not have any runtime side-effects. This means that the rule will report on these files as normal because changing an import to a type-only import will not have side-effects on the runtime code.

### Configuring `experimentalDecorators` and `emitDecoratorMetadata` for the rule

If you are using [type-aware linting](https://typescript-eslint.io/linting/typed-linting) then we will automatically infer your setup from your tsconfig and you should not need to configure anything.

Otherwise you can explicitly tell our tooling to analyze your code as if the compiler option was turned on by setting both [`parserOptions.emitDecoratorMetadata = true`](https://typescript-eslint.io/packages/parser/#emitdecoratormetadata) and [`parserOptions.experimentalDecorators = true`](https://typescript-eslint.io/packages/parser/#experimentaldecorators).

## Comparison with `importsNotUsedAsValues` / `verbatimModuleSyntax`

[`verbatimModuleSyntax`](https://www.typescriptlang.org/tsconfig#verbatimModuleSyntax) was introduced in TypeScript v5.0 (as a replacement for `importsNotUsedAsValues`). That being said we **do not** recommend that you use both this rule and `verbatimModuleSyntax` / `importsNotUsedAsValues` together at the same time.

As a comparison both this rule and `verbatimModuleSyntax` should _mostly_ behave in the same way. The main differences being:

- This rule includes an auto-fixer which can be applied from the ESLint CLI `--fix` flag or via your IDE's auto-fix-on-save feature.
- `verbatimModuleSyntax` will error on unused imports, where as this rule will ignore them
- `verbatimModuleSyntax` will work correctly when paired with both `experimentalDecorators` and `emitDecoratorMetadata`
- `verbatimModuleSyntax` will break your build if you have [`noEmitOnError`](https://www.typescriptlang.org/tsconfig#noEmitOnError) turned on
- `verbatimModuleSyntax` can change how imports are emitted from your build.
- Given the code `import { type T } from 'T';`:
- With `verbatimModuleSyntax: true` TS will emit `import {} from 'T'`.
- With `verbatimModuleSyntax: false` TS will emit nothing (it will "elide" the entire import).

Because there are some differences - using both this rule and `verbatimModuleSyntax` at the same time can lead to conflicting errors. As such we recommend that you only ever use one _or_ the other -- never both.

## When Not To Use It

Expand Down

0 comments on commit f3525d4

Please sign in to comment.