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

📚 noUndeclaredDependencies and useImportType report issues with output from TypeScript code actions #50

Closed
1 task done
chrisgrieser opened this issue Mar 10, 2024 · 7 comments

Comments

@chrisgrieser
Copy link

chrisgrieser commented Mar 10, 2024

Documentation URL

https://biomejs.dev/linter/rules/use-import-restrictions/

Description

Using typescript code actions Add missing imports results in code that is reported by biome. I am using the default typescript language server settings.

// biome reports `noUndeclaredDependencies`
import MyModule from "src/main"; // <- this line is auto-generated by "add missing imports"

// biome does not report`noUndeclaredDependencies` when changed to this:
import MyModule from "../main";
// biome reports `useImportType`
import MyModule from "module"; // <- this line is auto-generated by "add missing imports"
// …
let mod: MyModule;

While I think the first can be resolved by changing the typescript configuration to importModuleSpecifierPreference = relative (see here), I am not aware of any typescript setting to deal with the latter. (preferTypeOnlyAutoImports does that, see comment below)

Expectations

That a recommended rule (useImportType) reports output of typescript as an issue is quite irritating, and probably very confusing for less experienced users.

You could argue that this is somewhat a bug of those rules, but technically, the rules work as expected. Possible solutions to this:

A) Change the documentation

  1. Mention that these rules oppose the output of typescript code actions with default settings.
  2. Explain which typescript settings need to be changed to make the output of the code actions comply with biome's rules.

However, 2. is complicated by the fact that there is (to my knowledge) no typescript setting to make typescript add type imports instead of imports. (preferTypeOnlyAutoImports does that, as commented below)

Also, changing typescript configs is not always straightforward. Since tsserver is not "fully" open, users with an editor other than VSCode need to install special language server replacements to be able to configure the typescript.preferences, e.g. typescript tools.

That means: Quite a lot of effort to be able to comply with biome for new users.

B) Change rule behavior

Alternatively, the rule's behavior should be modified to be less strict. noUndeclaredDependencies could be modified to check if an import is simply project-relative, before reporting a violation.

Not sure how to deal with useImportType though. I'd consider demoting it from a recommended to a purely opt-in rule.

Code of Conduct

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

I have to take some time to write a complete answer. However, as a preliminary answer for useImportType: there is an LSP option named typescript.preferences.preferTypeOnlyAutoImports. This should partially solve your issue.

@ematipico ematipico transferred this issue from biomejs/biome Apr 16, 2024
@ematipico
Copy link
Member

@Conaclos I moved this issue here, but maybe I made a mistake. What do we need to do to address the issue and close it?

@Conaclos
Copy link
Member

What do we need to do to address the issue and close it?

We could add a note in the docs of useImportType about the vscode config typescript.preferences.preferTypeOnlyAutoImports.

Regarding noUndeclaredDependencies I am not sure what to do because the use of paths relative to the project root seems pretty rare? I don't know if there is an official doc about that.

@Conaclos
Copy link
Member

@chrisgrieser Could you share with us your current tsconfig file that allows to use import relative to the project root? This will help us to document this.

@chrisgrieser
Copy link
Author

@Conaclos tbh, part of the reason why I opened this issue was because I wasn't totally sure on what all the configs are, that you need to change to make biome "compatible" with typescript. And I have to admit that for now, I have simply disabled the respective biome rules 🙈

@Conaclos
Copy link
Member

Conaclos commented Jun 11, 2024

After some research, I think we are using the legacy Classic module resolution. Other resolution modes seems to disallow imports relative to project root. Because this mode is no longer recommended by TSC, I think we can close the issue.

@Conaclos Conaclos closed this as not planned Won't fix, can't repro, duplicate, stale Jun 11, 2024
@Sec-ant
Copy link
Member

Sec-ant commented Jun 11, 2024

I think the import specifier is controlled by this option. Maybe we should add it to the document? This is an LSP option:

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

No branches or pull requests

4 participants