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

Enhancement: [consistent-type-imports] option to only enable for internal dependencies #9047

Closed
4 tasks done
kuba-orlik opened this issue May 5, 2024 · 3 comments
Closed
4 tasks done
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin wontfix This will not be worked on

Comments

@kuba-orlik
Copy link

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/consistent-type-imports/

Description

The largest benefit I see in the consistent-type-imports rule is decreasing the amount of cyclic dependencies in the generated code.

I propose to add a new option to the consistent-type-imports rule that would only enable the checks for internal dependencies (where import path starts with a .).

Fail

import Shape from "./shapes.js"
import Container from "containers"

function create(): Container<Shape> | null{
    return null;
}

Pass

import type Shape from "./shapes.js"
import Container from "containers"

function create(): Container<Shape> | null{
    return null;
}

Additional Info

No response

@kuba-orlik kuba-orlik added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels May 5, 2024
@bradzacher
Copy link
Member

bradzacher commented May 5, 2024

Whats the motivation here? What benefit is there to just using it like this as opposed to just being consistent and treating all imports the same?

I don't see how there's broadly applicable value in being inconsistent and special casing this.

The largest benefit I see in the consistent-type-imports rule is decreasing the amount of cyclic dependencies in the generated code.

I strongly disagree. The benefit is to ensure your imports are properly qualified so that you can leverage non-type-aware transpilers or analysis tools to their fullest.

It allows you to creat self-documenting code that clearly describes the value-domain and type-domain imports.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for maintainers to take a look labels May 5, 2024
@kuba-orlik
Copy link
Author

Whats the motivation here? What benefit is there to just using it like this as opposed to just being consistent and treating all imports the same?

If the rule is enforced for all imports, then we get linter warnings on lines like

import {a,b,c,d} from "some-package";

If only some of those imports are for the type, and others for value, then we have to change it to

import type {b,d} from "some-package";
import {a,c} from "some-package";

That's extra work for our team that gives benefits less important (to us!) than what was the intention behind enabling this rule: decreasing cyclic dependencies across our codebase

@bradzacher
Copy link
Member

That's extra work for our team

But the rule has an autofixer for all cases - so you just run --fix and it fixes everything. So it's the same amount of work if it reports on one or one thousand imports.


The rule isn't intended to do anything around cyclic dependencies. If you're looking to reduce cyclic deps there are separate rules for that.


I don't see any broad applicability here and the motivation does not align with the intent of the rule. Thus I am going to reject this.

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale May 5, 2024
@bradzacher bradzacher added wontfix This will not be worked on and removed awaiting response Issues waiting for a reply from the OP or another party labels May 5, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants