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

rules not assignable to definitely typed eslint Rules which are used by config.plugin #172

Closed
spence-s opened this issue Mar 4, 2023 · 8 comments · Fixed by #173
Closed
Assignees
Labels
dependencies Pull requests that update a dependency file

Comments

@spence-s
Copy link

spence-s commented Mar 4, 2023

So the issue is this:

config.plugins is typed using ESLint.Plugin which contains a rules field which is typed: Record<string, ((...args: any[]) => any) | Rule.RuleModule> | undefined;

However, there are a lot of assignability issues between the way the definitely typed eslint package types rules and the way rules are typed here.

Is there a way we can fix this so that they are properly assignable?

Hope that issue was clear, thanks!

@Shinigami92
Copy link
Collaborator

Hope that issue was clear, thanks!

Not 100% yet, could you link me the definitely typed and maybe also provide a reproducible so I understand better what the issue is?

@spence-s
Copy link
Author

spence-s commented Mar 4, 2023

https://stackblitz.com/edit/typescript-zgntds?file=index.ts

import type { Rules } from 'eslint-define-config';
import type { Linter } from 'eslint';

const rules1: Rules = {};
const rules2: Linter.RulesRecord = {};

const rules: Linter.RulesRecord = {
  ...rules1,
  ...rules2,
};

Yes here is a minimal example showing the problem with a stackblitz. In strict mode Rules are not assignable to Linter.RulesRecord

@Shinigami92
Copy link
Collaborator

eslint-define-config uses

https://github.com/Shinigami92/eslint-define-config/blob/48e0ae4e6789056d4c8a4aefab45d537692f8cac/package.json#LL57C25-L57C26

Could you try downgrading and pin your @types/eslint to that? And try again?
The version did change really much and sometimes breaking changes are not covered in semver for TS types.
So maybe I just then need to bump the deps and recreate the Rules and everything. But when I have the time...

@spence-s
Copy link
Author

spence-s commented Mar 4, 2023

ahh I see - ok - yah that works, I can pin my version to be the same. thanks for the quick help @Shinigami92!

Going to leave this open for the community in case someone else runs into the same thing, but feel free to close if you want.

@Shinigami92 Shinigami92 added the dependencies Pull requests that update a dependency file label Mar 4, 2023
@Shinigami92 Shinigami92 self-assigned this Mar 4, 2023
@Shinigami92
Copy link
Collaborator

ahh I see - ok - yah that works, I can pin my version to be the same. thanks for the quick help @Shinigami92!

Going to leave this open for the community in case someone else runs into the same thing, but feel free to close if you want.

Perfect 👍
Thanks for the info
I will act when I have the time

@Shinigami92
Copy link
Collaborator

I had a first look and it takes more time than just some minutes

I need to update many rules especially the generation for eslint-plugin-{node/N} and there seems not to be two changes that might have made my own type-defs incompatible with eslint new defs

// These are my own types
export type RuleSeverity = 'off' | 'warn' | 'error' | 0 | 1 | 2;

export type RuleConfig<Options extends unknown[] = unknown[]> =
  | RuleSeverity
  | [RuleSeverity, ...Options];


// These are the new eslint types
type Prepend<Tuple extends any[], Addend> = ((_: Addend, ..._1: Tuple) => any) extends (..._: infer Result) => any
    ? Result
    : never;

type Severity = 0 | 1 | 2;

type RuleLevel = Severity | "off" | "warn" | "error";
type RuleLevelAndOptions<Options extends any[] = any[]> = Prepend<Partial<Options>, RuleLevel>;

type RuleEntry<Options extends any[] = any[]> = RuleLevel | RuleLevelAndOptions<Options>;
  1. it extends any[] instead of unknown[]
  2. It uses Partial for Options

I assume one of these both breaks it
But also that way my types are stronger!

Maybe I could even harden the types on https://github.com/DefinitelyTyped/DefinitelyTyped/blob/808bf075ca716aab85e3e317b49d8638cacc23a5/types/eslint/index.d.ts#L717-L719 instead of loosing my types

The biggest problem!: I cannot use @types/eslint in my src!!! Because then I would enforce @types/eslint to be installed for everyone using my plugin. But it is "just" a devDependency and so I would need to sync my types with them somehow.

... I need more time to investigate this and dig deeper (so sadly maybe not today)

@Shinigami92
Copy link
Collaborator

@spence-s would you like to give me feedback if everything is working as needed?

@spence-s
Copy link
Author

spence-s commented Mar 9, 2023

@Shinigami92 - I actually changed my approach a bit so that this libs types override anything from eslint so I no longer have the problem at all. (even without the patch) - I do appreciate your quick attention to this though! 🙌🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants