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] Using with --fix changes constructor imports to type imports in nest and angular #3630

Closed
3 tasks done
yharaskrik opened this issue Jul 12, 2021 · 3 comments · Fixed by #4646
Closed
3 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: parser Issues related to @typescript-eslint/parser

Comments

@yharaskrik
Copy link

  • 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

https://github.com/yharaskrik/eslint-type-import-repro

yarn nx affected:lint --fix

Expected Result

I would expect that concrete classes that are needed for dependency injection, ClassType, would be left as non-type imports and the types that are not needed to be concrete are changed to type only imports, InterfaceType. I realize this is probably framework dependant since not everything uses DI and needs concrete classes at runtime.

Actual Result

As it stands the imports in app.service.ts are not using type imports. when running yarn nx affected:lint --fix (which uses eslint and @typescript-eslint under the hood) it changes the import to import type { ClassType, InterfaceType } from './type'; which then breaks the nest server as the ClassType is a concrete class that is needed for dependency injection.

Versions

package version
@typescript-eslint/eslint-plugin ^4.28.2
@typescript-eslint/parser ^4.28.2
TypeScript ~4.2.4
ESLint 7.22.0
node 12.22.1
@yharaskrik yharaskrik added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Jul 12, 2021
@bradzacher
Copy link
Member

bradzacher commented Jul 12, 2021

We have support for this, but the issue here is that you don't use type-aware linting (at least in your repro you don't), so we can't detect if you've turned on emitDecoratorMetadata, so we default to off.

We don't currently have a mechanism to allow you to turn this on via parserOptions.
Happy to accept a PR to expose the emitDecoratorMetadata flag in parserOptions!


Why do we do detection for this? Because the meaning is vastly different if you have emitDecoratorMetadata on vs if you have it off - as you can see. If we defaulted to on, then most usecases would do incorrect analysis. It's really only angular that uses decorator metadata, which is a relatively small fraction of linting usecases.

@bradzacher bradzacher added enhancement New feature or request package: parser Issues related to @typescript-eslint/parser and removed package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Jul 12, 2021
@yharaskrik
Copy link
Author

Thanks for your response @bradzacher! How would I go about turning on type-aware linting?

I can understand you probably wouldn't want that turned on by default of course! For us our whole stack is Nest+Angular (which both use decorators). I can see if I can work on a PR at some point, is there a work around for now that I can try (I would assume manually use type-aware linting?)?

@bradzacher
Copy link
Member

is there a work around for now that I can try

Yup - the only option is to turn on type-aware linting:
https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/TYPED_LINTING.md

@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
@bradzacher bradzacher linked a pull request Mar 11, 2022 that will close this issue
3 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2022
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 feature or request package: parser Issues related to @typescript-eslint/parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants