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

useImportType breaks class based dependency injection in Nest.JS #2003

Closed
1 task done
ssilve1989 opened this issue Mar 8, 2024 · 4 comments
Closed
1 task done

useImportType breaks class based dependency injection in Nest.JS #2003

ssilve1989 opened this issue Mar 8, 2024 · 4 comments

Comments

@ssilve1989
Copy link

Environment information

CLI:
  Version:                      1.6.0
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v20.6.1"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "pnpm/8.15.4"

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 false

Workspace:
  Open Documents:               0

Rule name

lint/style/useImportType

Playground link

https://codesandbox.io/p/devbox/peaceful-bird-glx5h2

Expected result

It should not apply import type to class types where they are used for dependency injection. The generated TS when using import type on the class import results in

exports.AppController = AppController = __decorate([
    (0, common_1.Controller)(),
    __metadata("design:paramtypes", [Function])
], AppController);

which is lacking the necessary metadata information, whereas without import type it generates

exports.AppController = AppController = __decorate([
    (0, common_1.Controller)(),
    __metadata("design:paramtypes", [app_service_1.AppService])
], AppController);

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@Conaclos
Copy link
Member

This could be the same issue as #2038. I am waiting more details to decide if they are instances of the sane issue.

ianzone added a commit to ianzone/lambda-nest that referenced this issue Mar 22, 2024
@joshmeads
Copy link

+1 on this. Was going mad trying to find out why my app suddenly broke. I LOVE this rule, it's something I've been wanting it for years but I was suddenly thrown into a NestJS project and it took me by surprise.

Would it be possible to have an option to allow classes?

As there is no option in Biome (that I can see) to disable autofix for a single rule it makes it rather difficult to work with.

@mallowigi
Copy link

Just started giving a shot to Biome and immediately noticed this. This makes it a no-go for Biome as far as I'm standing here.

@Conaclos
Copy link
Member

Conaclos commented Mar 31, 2024

useImportType is largely inspired from TypeScript Eslint's consistent-type-imports. Until their last release, TS ESlint tried to handle this specific case. However, they changed their mind, and the last version of TS ESlint now rejects this like Biome's useImportType. The main reasons are:

  • conflicts with TypeScript's isolatedModules,
  • TS decorators are based on a legacy and retired proposal and are on the verge of deprecation.

The only exception they have introduced is to completely disable the rule in a file containing at least one decorator if the experimentalDecorators and emitDecoratorMetadata options are enabled.

Based on this new knowledge, I think it would be a mistake to waste time on this. I recommend that users simply disable the rule in projects that use frameworks that rely on an outdated and never-standardised syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants