-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Configs: Make sure that the stylistic config doesn't conflict with isolatedDeclarations #10001
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
Comments
The stylistic config is, well, stylistic. The opinions are what the majority of TS codebases would agree on based on readability, conciseness, and consistency. Now there would always be TS options that make you write code in a different style, and ID often enforces a style that's foreign to non-ID codebases. I don't think we should change the stylistic config to accommodate ID; rather, we could make a config that turns off rules that conflict with ID (for example, in the current world, ID is basically the same as However, I'm coming over from microsoft/TypeScript#60010, and I think if microsoft/TypeScript#60010 gets accepted, this issue may be completed obsoleted. |
That makes sense to me. I think the thrust of my issue is that ID users should have some way to turn on "stylistic" linting without needing to make the individual rule decisions themselves. There are multiple ways to provide that, but having a "stylistic for ID projects" config seems like a perfectly cromulent way to achieve that (as would microsoft/TypeScript#60010, although I'm less clear how feasible that would be to land) Minor nit: The ID option currently requires type annotations only for exported declarations, so it's a still a style call how to handle local declarations and non-exported module-level declarations. |
As awesome and beneficial and exciting as Broadly lessening the usefulness of However, lint rules do have the ability to read compiler options. I think we can consider it a bug that Proposal (cc @typescript-eslint/triage-team): how about we retarget this issue & marking as accepting PRs to make the rule respect |
I'm personally at a -1 At best I think we could offer a config to layer on top to disable any rules that conflict with it. But I would probably say "this requirement applies to less than 10% of configs so it's not worth us building a config to make it easier". |
What does this look like concretely? Issue an error with incompatible options, like, eg, prefer-nullish-coalescing without strictNullChecks? Or something else? |
I was thinking that |
Should ID users just use the 'type-annotation' option, though, rather than the 'constructor' option? Would the 'constructor' option with ID ever report under this proposal? |
My 2c is that ID users are rare enough that we shouldn't look to support it aside from making it possible for rules to be used with it when it makes sense. We should just make sure that rules that could work with it have the option to do so. But I don't think we should change the config around it. |
ID is only relevant for export const checked = new Set<string>();
export async function main() {
const ignored = new Set<string>();
}
Sounds like we're generally 👍 on this - so from my interpretation, the only next step would be to make |
Ah! gotcha, that was the missing part for me. Thanks!
👍 from me |
To determine if ID is on you need type-aware linting so we can get the tsconfig. But both of these rules are not type-aware. Which would mean we need to add a parser option which the user can manually set or we prefill if type-aware linting is on (just like the emitDecoratorMetadata setup). Seems like a lot of effort for what is right now a pretty niche usecase, TBH. |
Is it more complex than #10499? I think it's fair to assume ID is going to slowly get more popular over time. Especially given tools like oxbuild and unplugin-isolated-decl getting more popular that generate tl;dr: I'm pretty solidly +1 on taking this cost. |
It looks like we have consensus on not changing the stylistic config itself to support isolatedDeclarations (ID). Multiple of us are leaning towards instead having individual rules understand ID and not produce conflicting reports. So I'm going to go ahead and close this issue out in favor of splitting out:
Thanks all! |
Before You File a Proposal Please Confirm You Have Done The Following...
Description
The stylistic config includes the https://typescript-eslint.io/rules/consistent-generic-constructors/ rule, which (in it's default configuration) conflicts with the requirements of TS 5.5's new isolatedDeclarations mode.
In isolatedDeclarations mode, the following code is illegal:
giving an error: "Variable must have an explicit type annotation with --isolatedDeclarations."
TypeScript Playground
Whereas in ESLint, the opposite pattern is disallowed by the
consistent-generic-constructors
from the stylistic config:gives a different error: "The generic type arguments should be specified as part of the constructor type arguments."
ESLint Playground
Ideally, users using both the stylistic config and the isolatedDeclartions tsconfig option should have a less frustrating out-of-the-box experience. I think ESLint devs are better positioned to make the call on what that should look like, but I few options that I can think of would be:
consistent-generic-constructors
by default in the stylistic config, orconsistent-generic-constructors
to avoid running on isolatedDeclarations projects, orImpacted Configurations
stylistic
Additional Info
No response
The text was updated successfully, but these errors were encountered: