Skip to content

chore(eslint-plugin): [no-invalid-void-type] fix Options typing to reflect minItems: 1 #8312

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 3, 2024
Merged

chore(eslint-plugin): [no-invalid-void-type] fix Options typing to reflect minItems: 1 #8312

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/no-invalid-void-type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { getSourceCode } from '@typescript-eslint/utils/eslint-utils';
import { createRule } from '../util';

interface Options {
allowInGenericTypeArguments?: string[] | boolean;
allowInGenericTypeArguments?: [string, ...string[]] | boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this makes sense technically 😄. But working with types like this can be irksome since you have to do it consistently. Is there an end-user reason for this change?

I'm not opposed to it, just curious how it came up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh hi 😄.

It's true that this isn't a random PR out of nowhere. I was reviewing my typescript-eslint rule typings (I have only 4 of them and they are for a no-deadline personal project, which means that there's room for everything + the kitchen sink in it) and originally I had something like the following:

'@typescript-eslint/no-invalid-void-type'?: Linter.RuleEntry<[{
  allowInGenericTypeArguments?: string[] | boolean;
  allowAsThisParameter?: boolean;
}]>;

I checked no-invalid-void-type's documentation and noticed that the typing for allowInGenericTypeArguments had tightened up. For some reason (probably because I wanted to find out whether I can get rule typings directly from typescript-eslint source) I checked the rule source and noticed immediately that the Options typing for it was out-of-date, largely because it's at the top of the file I guess.

Is there an end-user reason for this change?

I figured that the out-of-date typing would affect DX for typescript-eslint itself, but after creating a test script in packages/eslint-plugin/src (of the typescript-eslint repo) and doing:

import rules from './rules';
const aVar = rules['no-invalid-void-type'].create(/* ??? */);

I felt I was in a bit too deep to be comfortable, so instead of figuring out how the create method fits in the whole scheme of things, and then doing a "Complex Bug With a Reproduction" issue with my fork of typescript-eslint as the repo, I figured that I'd just take my chances and go straight to a PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, makes sense. It looks like this is the only minItems: 1 in a non-deprecated rule, so I guess you're setting a good precedent. Nice 😄

allowAsThisParameter?: boolean;
}

Expand Down