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

Type Parameter name TPublicMethod must match one of the following formats: StrictPascalCase #41

Closed
fregante opened this issue Oct 3, 2021 · 10 comments

Comments

@fregante
Copy link
Member

fregante commented Oct 3, 2021

It seems that the config allows types named "TSomething":

// Type parameter name should either be `T` or a descriptive name.

but I'm getting this error:

  274:3   error    Type Parameter name `TMethod` must match one of the following formats: StrictPascalCase                                                      @typescript-eslint/naming-convention
  275:3   error    Type Parameter name `TPublicMethod` must match one of the following formats: StrictPascalCase                                                @typescript-eslint/naming-convention
  278:3   error    Type Parameter name `TType` must match one of the following formats: StrictPascalCase                                                        @typescript-eslint/naming-convention
  279:3   error    Type Parameter name `TMethod` must match one of the following formats: StrictPascalCase                                                      @typescript-eslint/naming-convention
  280:3   error    Type Parameter name `TPublicMethod` must match one of the following formats: StrictPascalCase                                                @typescript-eslint/naming-convention

See run https://github.com/pixiebrix/webext-messenger/actions/runs/1300974580 for pixiebrix/webext-messenger#35.

@fregante
Copy link
Member Author

fregante commented Oct 3, 2021

If this is intentional, I was under the impression that TSomething is the convention to name a local parameter, like Array<TSomething extends Something>, so I'm not sure of what the suggestion is.

Example: https://github.com/pixiebrix/webext-messenger/blob/1e6f62e8a85cafa18a5d5124fa2de8da1b25331d/index.ts#L59-L61

@sindresorhus
Copy link
Member

I intentionally don't allow T-prefixed type parameter names as I see it as an anti-pattern. Just use the pure name or Type suffix if it conflicts.

https://til.hashrocket.com/posts/cfnlyzxcrc-do-not-prefix-typescript-interface-names

@fregante
Copy link
Member Author

fregante commented Oct 3, 2021

The article is about ISomething, which I don’t like either. TSomething seems used a lot and I don’t think enforcement is worth it. TypeScript generics are verbose as is, I don't really want to make them unnecessarily longer.

https://github.com/pixiebrix/webext-messenger/blob/1e6f62e8a85cafa18a5d5124fa2de8da1b25331d/index.ts#L264-L281

@sindresorhus
Copy link
Member

The same arguments apply though. You can see that the TS docs does not use the T prefix either: https://www.typescriptlang.org/docs/handbook/2/generics.html#working-with-generic-type-variables

@fregante
Copy link
Member Author

fregante commented Oct 4, 2021

I don’t think that page helps your argument since it contains

  • Type extends Lengthwise
  • A extends Animal

as 2 of the 3 examples that use extends in generics. 😃

The main issue is that extends Foo usually refers to a well-defined "global" type, and the TFoo refers to the local version of it. Which is really still a Foo, but scoped to the function. Other than calling it LocalFoo or ScopedFoo, I don't see many alternatives — and are those any better than the conventional TFoo? I'd argue they aren't

@sindresorhus
Copy link
Member

sindresorhus commented Oct 4, 2021

Before:

function getMethod<
  TType extends keyof MessengerMethods,
  TMethod extends MessengerMethods[TType],
  TPublicMethod extends PublicMethod<TMethod>
>(
  type: TType,
  options: { isNotification: true }
): SetReturnType<TPublicMethod, void>;

After:

function getMethod<
  Type extends keyof MessengerMethods,
  Method extends MessengerMethods[Type],
  PublicMethodType extends PublicMethod<Method>
>(
  type: Type,
  options: { isNotification: true }
): SetReturnType<PublicMethodType, void>;

Conflicts are rare in practice.

@fregante
Copy link
Member Author

fregante commented Oct 4, 2021

If you want to look at what Microsoft does, check their libs, they all use single-letters:

@fregante
Copy link
Member Author

fregante commented Oct 4, 2021

The first 2 are fine, so I'll try following it, but PublicMethodType isn't better than TPublicMethod — but maybe it's just a casualty/exception of the improved 2 other types.

@sindresorhus
Copy link
Member

I generally use T or Value if there's only one generic type parameter, unless it's not immediately clear what T is, then I use a more descriptive name. If there are more than one, I always use descriptive names.

@EdJoPaTo
Copy link

EdJoPaTo commented Oct 4, 2021

Personal opinion: in most cases with a conflict the naming isnt descriptive enough for a casual human reader to understand their differences. So in most cases the improved names are also easier to understand afterwards.

If it is the special case of something TypeScript is able to differentiate between them and uses the "inner" specified one. But using a better name than just T as a prefix is also better here in my opinion.

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants