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-import] flags non-type-only imported type as a value import when used in a decorator #3108

Closed
vegerot opened this issue Feb 24, 2021 · 4 comments
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@vegerot
Copy link
Contributor

vegerot commented Feb 24, 2021

As a continuation from #2972 (comment), I was wondering if @bradzacher would be able to comment on whether or not vuejs/vue-cli#3145 (comment) is a bug in vue or in typescript-eslint

@vegerot vegerot added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Feb 24, 2021
@bradzacher
Copy link
Member

This is something I didn't know about typescript's decorator metadata.
It does use type information to change the emitted code - which is rather surprising.

If I write this code:

declare function Component(...args: any[]): any;
declare function namespace(...args: any[]): any;

@Component
export default class Comp {
 @namespace('stuff').Action('fetch') private readonly fetchStuff!: ActionMethod;
}

Then TS will see the value does not exist in scope, and attempt to maybe reference the value:

    __metadata("design:type", typeof (_a = typeof ActionMethod !== "undefined" && ActionMethod) === "function" ? _a : Object)

If I add an import to a class (eg import {Component as ActionMethod} from 'react';), TS will emit the same line

If I declare a class within the file (class ActionMethod { }), TS will emit a slightly different line

    __metadata("design:type", ActionMethod)

If I declare a type within the file, TS will emit code depending on what the type is:

So far this is all well and good - TS can and should use the information from within the module to emit code.

If, however, I import a type (import {ActionMethod} from 'vuex';), TS will detect that it is a type, evaluate it's type and emit that:

    __metadata("design:type", Function)

Which is really surprising - it's the first time I've seen it use type information from an imported thing thing to change the emitted code.

This is obviously not really compatible with non-type-aware tools that work in an isolated model.

@vegerot
Copy link
Contributor Author

vegerot commented Feb 24, 2021

Wow that is very interesting. Thank you for linking the playgrounds and discussion. I learned a lot about how TypeScript emits decorator metadata. In this case, what should be the solution in vue class components?

@bradzacher bradzacher changed the title [consistent-type-import] question for @bradzacher [consistent-type-import] flags non-type-only imported type as a value import when used in a decorator Feb 24, 2021
@bradzacher bradzacher added bug Something isn't working and removed triage Waiting for maintainers to take a look labels Feb 24, 2021
@bradzacher
Copy link
Member

it seems like a bug in the vue tooling (or whatever tooling it's built on top of).
if you import a type without making it a type-only import then build tool should work as expected.

There is a separate bug in the lint rule because it does not understand that TS understands that the imported type is a type reference only. I don't know if we can fix this, however, because it would require using type information.

@JoshuaKGoldberg
Copy link
Member

I'm going to go ahead and mark this as non-actionable since it's a bug in other packages:

  • It would be a breaking change and inconvenience to the consistent-type-import to require type information for this one specific case
  • TypeScript relying on types in emit is highly unusual and not a scenario generally recommended for tools
  • There is also an issue in Vue tooling

Please post back if you have a proposal for how we could solve this issue in typescript-eslint

@JoshuaKGoldberg JoshuaKGoldberg added the external This issue is with another package, not typescript-eslint itself label Feb 25, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

3 participants