Skip to content

fix(eslint-plugin): [consistent-generic-constructors] ignore when constructor is typed array #10477

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,37 @@ const set: Set<string> = new Set<string>();
</TabItem>
</Tabs>

### `ignore`

{/* insert option description */}

Some constructors have different type signatures between their type and value, for example like Uint8Array.

<Tabs>
<TabItem value="❌ Incorrect">

```ts option='"constructor"'
// Incorrect because Uint8Array has deffenrent type signature and not in ignorelist
let a: Uint8Array<ArrayBufferLike> = new Uint8Array();

// Incorrect because type arguments appear in type-annotation and not in ignorelist
let a: UserConstructor<Type> = new UserConstructor();
```

</TabItem>
<TabItem value="✅ Correct">

```ts option='"constructor", { "ignore": ["Uint8Array", "UserConstructor"] }' showPlaygroundButton
// Correct because Uint8Array has deffenrent type signature but are included in the ignorelist.
let a: Uint8Array<ArrayBufferLike> = new Uint8Array();

// Correct because type arguments appear in type-annotations but are included in the ignorelist.
let a: UserConstructor<Type> = new UserConstructor();
```

</TabItem>
</Tabs>

## When Not To Use It

You can turn this rule off if you don't want to enforce one kind of generic constructor style over the other.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ import { AST_NODE_TYPES } from '@typescript-eslint/utils';

import { createRule, nullThrows, NullThrowsReasons } from '../util';

export type MessageIds = 'preferConstructor' | 'preferTypeAnnotation';
export type Options = ['constructor' | 'type-annotation'];
type MessageIds = 'preferConstructor' | 'preferTypeAnnotation';
type Options = [
'constructor' | 'type-annotation',
{
ignore?: string[];
}?,
];

export default createRule<Options, MessageIds>({
name: 'consistent-generic-constructors',
Expand All @@ -29,10 +34,24 @@ export default createRule<Options, MessageIds>({
description: 'Which constructor call syntax to prefer.',
enum: ['type-annotation', 'constructor'],
},
{
type: 'object',
additionalProperties: false,
properties: {
ignore: {
type: 'array',
description:
'A list of constructor names to ignore when enforcing the rule.',
Copy link
Member

Choose a reason for hiding this comment

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

IMO if we do go with a list then we'd want to use TypeOrValueSpecifier. Otherwise it'll have the same issue as #10477 (comment) with coincidentally matching names.

But, I'm not sure this is the right step. I started a thread later in this file about it.

items: {
type: 'string',
},
},
},
},
],
},
defaultOptions: ['constructor'],
create(context, [mode]) {
defaultOptions: ['constructor', {}],
create(context, [mode, options]) {
return {
'VariableDeclarator,PropertyDefinition,AccessorProperty,:matches(FunctionDeclaration,FunctionExpression) > AssignmentPattern'(
node:
Expand Down Expand Up @@ -77,7 +96,8 @@ export default createRule<Options, MessageIds>({
lhs &&
(lhs.type !== AST_NODE_TYPES.TSTypeReference ||
lhs.typeName.type !== AST_NODE_TYPES.Identifier ||
lhs.typeName.name !== rhs.callee.name)
lhs.typeName.name !== rhs.callee.name ||
options?.ignore?.includes(lhs.typeName.name))
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg Mar 31, 2025

Choose a reason for hiding this comment

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

Threading #10477 (comment):

Should I add TypedArray to the default option?

It looks like we don't have consensus on this yet?

One concern with having default values in the default list is that it gets inconvenient to add to the list rather than replace it. If the default type includes, say, Proxy and all the Uint*Arrays, then that's a lot of manual re-typing for folks to write if they want to keep ignoring them.

We're previously had options like ban-types > extendDefaults to get around this, but they're kind of clunky.

@Josh-Cena I think what's confusing me here is:

  • What benefit is there to reporting on Uint8Array and similar? I.e.: in what situations would one want the rule to report on them?
  • If there are none, why not hardcode the rule to always ignore them?

cc @kirkwaiblinger

) {
return;
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ class A {
`
const a = function (a: Foo = new Foo<string>()) {};
`,
{
code: 'let a: Uint8Array<ArrayBufferLike> = new Uint8Array();',
options: ['constructor', { ignore: ['Uint8Array'] }],
},
// type-annotation
{
code: 'const a = new Foo();',
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.