ci(eslint): add rule to encourage generic naming convention as `^(T|T[A-Z][A-Za-z]+)$` #6684

manudeli · 2024-01-11T18:00:50Z

@TkDodo I saw your post in X and @juliusmarminge's solution by @typescript-eslint/naming-convention with regex
I want to add it in this repository

I've been using this rule in various repositories for quite some time, and I really liked the fact that when defining a new generic, it encouraged me not to just name it T. It make coworker and interface user feel better DX.

@juliusmarminge Thanks for sharing your cool solution too.

vercel · 2024-01-11T18:00:54Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
query	⬜️ Ignored (Inspect)	Visit Preview		Jan 28, 2024 5:05pm

nx-cloud · 2024-01-11T18:01:44Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 3be51b3. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 1 target

nx affected --targets=test:eslint,test:lib,test:types,test:build,build --exclude=examples/**

Sent with 💌 from NxCloud.

codesandbox-ci · 2024-01-11T18:03:22Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3be51b3:

Sandbox	Source
@tanstack/query-example-angular-basic	Configuration
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

.eslintrc.cjs

TkDodo · 2024-01-13T22:20:16Z

.eslintrc.cjs

+        selector: 'typeParameter',
+        format: ['PascalCase'],
+        leadingUnderscore: 'allow',
+        custom: {
+          regex: '^(T|\\$)[A-Z][a-zA-Z]+[0-9]*$',
+          match: true,


hmm why would we allow leading underscores on a type param? I've set this in other projects to:

{ selector: 'typeParameter', format: ['PascalCase'], leadingUnderscore: 'forbid', trailingUnderscore: 'forbid', custom: { regex: '^(T|\\$)([A-Z][a-z]+)*$', match: true, }, },

hmm why would we allow leading underscores on a type param?

Oh thanks, I didn't care these options(leadingUnderscore, trailingUnderscore) mistakely.
I banned underscores, and simplified regex

like below

{ selector: 'typeParameter', format: ['PascalCase'], leadingUnderscore: 'forbid', trailingUnderscore: 'forbid', custom: { regex: '^([A-Z]|T[A-Z][A-Za-z]+)$'', match: true, }, }

.eslintrc.cjs

manudeli · 2024-01-14T08:50:57Z

could we try to error it and just rename the type parameters if necessary? This should be non breaking ...

@TkDodo I will resolve eslint errors soon. wait a minutes please.

manudeli · 2024-01-14T09:11:08Z

could we try to error it and just rename the type parameters if necessary? This should be non breaking ...

@TkDodo I will resolve eslint errors soon. wait a minutes please.

I tried to resolve eslint errors, but there was too many cases that don't need this rule like below. 🥲
How about this rule as warning now? This rule with warning also will make us follow this rule too.

export type MaybeRef<T> = Ref<T> | T // isn't it T also good? I think warning is enough for this case

export type MaybeRefOrGetter<T> = MaybeRef<T> | (() => T)

...smillar cases

or this rule as error in each files with eslint-disable if need?

TkDodo · 2024-01-14T09:25:59Z

T alone should be valid according to the regex, no?

manudeli · 2024-01-14T09:32:30Z

T alone should be valid according to the regex, no?

Ah~, I thought T should be banned before.
Then K, X, Y should be valid right too?

codecov-commenter · 2024-01-14T16:59:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b32ad24) 41.78% compared to head (3be51b3) 41.78%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6684   +/-   ##
=======================================
  Coverage   41.78%   41.78%           
=======================================
  Files         178      178           
  Lines        7017     7017           
  Branches     1421     1421           
=======================================
  Hits         2932     2932           
  Misses       3722     3722           
  Partials      363      363

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

manudeli · 2024-01-14T17:37:27Z

@TkDodo sorry, My reply was late because I didn't know much about regex and was studying it. I update regex for this rule as ^([A-Z]|T[A-Z][A-Za-z]+)$

This rule will make contributors use only below.

okay

alone T, U, V, etc. is okay
TQueryKey, TQueryFnData is okay

not okay

KQueryData, Result, Depth, State, DefinedQueryObserver, Runner, Args, TaggedQueryKey, Type, Value, etc. is banned from now. these should be started with T

And I updated codes not following this rule

ps. When I test this rule, I used this tool(https://rex.antfu.me/) made by @antfu. so cool thanks anthony fu!

Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>

manudeli · 2024-01-21T03:10:23Z

@TkDodo check this again please

TkDodo · 2024-01-27T15:04:23Z

@manudeli I would prefer it if T would be the only allowed single character type parameter. This would make sure that one function can have T as a generic, but as soon as you add a second, you can't make <T,U> - you need to give meaningful names.

Imo one-character generics are best if there is only one on a given type / function. With two or more, you always need to now what they stand for ...

manudeli · 2024-01-27T15:10:37Z

@manudeli I would prefer it if T would be the only allowed single character type parameter. This would make sure that one function can have T as a generic, but as soon as you add a second, you can't make <T,U> - you need to give meaningful names.

Imo one-character generics are best if there is only one on a given type / function. With two or more, you always need to now what they stand for ...

Ahha, I understood your intention. and I agree your opinion. then I'll change it as following you want

…e strictly

manudeli · 2024-01-27T16:08:53Z

@TkDodo I update rule as ^(T|T[A-Z][A-Za-z]+)$ from ^([A-Z]|T[A-Z][A-Za-z]+)$ and I resolved error from this rule in some code. We can check change in 31140ac

TkDodo · 2024-01-28T16:58:27Z

can you please fix the conflicts ?

manudeli · 2024-01-28T17:10:06Z

can you please fix the conflicts ?

@TkDodo Sure, I resolved conflict now

# Overview  TanStack/query#6684 ## PR Checklist - [x] I did below actions if need 1. I read the [Contributing Guide](https://github.com/suspensive/react/blob/main/CONTRIBUTING.md) 2. I added documents and tests.

manudeli commented Jan 11, 2024

View reviewed changes

.eslintrc.cjs Outdated Show resolved Hide resolved

manudeli marked this pull request as ready for review January 11, 2024 18:13

manudeli changed the title ~~ci(eslint): add rule to force generic naming convention~~ ci(eslint): add rule to force generic naming convention as '^(T|\\$)[A-Z][a-zA-Z]+[0-9]*$' Jan 11, 2024

manudeli changed the title ~~ci(eslint): add rule to force generic naming convention as '^(T|\\$)[A-Z][a-zA-Z]+[0-9]*$'~~ ci(eslint): add rule to force generic naming convention as ^(T|\\$)[A-Z][a-zA-Z]+[0-9]*$ Jan 11, 2024

manudeli changed the title ~~ci(eslint): add rule to force generic naming convention as ^(T|\\$)[A-Z][a-zA-Z]+[0-9]*$~~ ci(eslint): add rule to encourage generic naming convention as ^(T|\\$)[A-Z][a-zA-Z]+[0-9]*$ Jan 11, 2024

TkDodo reviewed Jan 13, 2024

View reviewed changes

manudeli changed the title ~~ci(eslint): add rule to encourage generic naming convention as ^(T|\\$)[A-Z][a-zA-Z]+[0-9]*$~~ ci(eslint): add rule to encourage generic naming convention as ^([A-Z]|T[A-Z][A-Za-z]+)$ Jan 14, 2024

manudeli requested a review from TkDodo January 14, 2024 17:40

manudeli and others added 5 commits January 21, 2024 00:38

ci(eslint): add rule to force generic naming convention

1fdb49b

chore: reflect review

e86ea77

Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>

fix(eslint): accept A-Z alone too

dbb6e00

chore: update rule regex

458be3e

chore: update rule's regex

25177d2

manudeli force-pushed the ci/eslint/naming-convention-TXXX-for-generic branch from 0f245e4 to 25177d2 Compare January 20, 2024 15:39

manudeli added 2 commits January 21, 2024 06:15

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

a03b844

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

07ca9e5

manudeli added 4 commits January 22, 2024 20:57

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

42fb81f

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

4b19ede

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

bcecb63

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

bfe9443

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

77475ec

fix(eslint): update typeParameter rule as ^(T|T[A-Z][A-Za-z]+)$ mor…

31140ac

…e strictly

manudeli changed the title ~~ci(eslint): add rule to encourage generic naming convention as ^([A-Z]|T[A-Z][A-Za-z]+)$~~ ci(eslint): add rule to encourage generic naming convention as ^(T|T[A-Z][A-Za-z]+)$ Jan 27, 2024

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

76e961f

manudeli added 2 commits January 29, 2024 02:02

Merge branch 'main' into ci/eslint/naming-convention-TXXX-for-generic

10a40e9

chore: resolve prettier error

3be51b3

TkDodo merged commit c774772 into TanStack:main Jan 28, 2024

manudeli mentioned this pull request Feb 14, 2024

ci(eslint): update typeParameter rule toss/suspensive#732

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

ci(eslint): add rule to encourage generic naming convention as `^(T|T[A-Z][A-Za-z]+)$` #6684

ci(eslint): add rule to encourage generic naming convention as `^(T|T[A-Z][A-Za-z]+)$` #6684

manudeli commented Jan 11, 2024 •

edited

Loading

vercel bot commented Jan 11, 2024 •

edited

Loading

nx-cloud bot commented Jan 11, 2024 •

edited

Loading

codesandbox-ci bot commented Jan 11, 2024 •

edited

Loading

TkDodo Jan 13, 2024

manudeli Jan 14, 2024 •

edited

Loading

manudeli commented Jan 14, 2024

manudeli commented Jan 14, 2024 •

edited

Loading

TkDodo commented Jan 14, 2024

manudeli commented Jan 14, 2024 •

edited

Loading

codecov-commenter commented Jan 14, 2024 •

edited

Loading

manudeli commented Jan 14, 2024 •

edited

Loading

manudeli commented Jan 21, 2024

TkDodo commented Jan 27, 2024

manudeli commented Jan 27, 2024 •

edited

Loading

manudeli commented Jan 27, 2024 •

edited

Loading

TkDodo commented Jan 28, 2024

manudeli commented Jan 28, 2024

ci(eslint): add rule to encourage generic naming convention as ^(T|T[A-Z][A-Za-z]+)$ #6684

ci(eslint): add rule to encourage generic naming convention as ^(T|T[A-Z][A-Za-z]+)$ #6684

Conversation

manudeli commented Jan 11, 2024 • edited Loading

vercel bot commented Jan 11, 2024 • edited Loading

nx-cloud bot commented Jan 11, 2024 • edited Loading

☁️ Nx Cloud Report

codesandbox-ci bot commented Jan 11, 2024 • edited Loading

TkDodo Jan 13, 2024

Choose a reason for hiding this comment

manudeli Jan 14, 2024 • edited Loading

Choose a reason for hiding this comment

manudeli commented Jan 14, 2024

manudeli commented Jan 14, 2024 • edited Loading

TkDodo commented Jan 14, 2024

manudeli commented Jan 14, 2024 • edited Loading

codecov-commenter commented Jan 14, 2024 • edited Loading

Codecov Report

manudeli commented Jan 14, 2024 • edited Loading

okay

not okay

manudeli commented Jan 21, 2024

TkDodo commented Jan 27, 2024

manudeli commented Jan 27, 2024 • edited Loading

manudeli commented Jan 27, 2024 • edited Loading

TkDodo commented Jan 28, 2024

manudeli commented Jan 28, 2024

ci(eslint): add rule to encourage generic naming convention as `^(T|T[A-Z][A-Za-z]+)$` #6684

ci(eslint): add rule to encourage generic naming convention as `^(T|T[A-Z][A-Za-z]+)$` #6684

manudeli commented Jan 11, 2024 •

edited

Loading

vercel bot commented Jan 11, 2024 •

edited

Loading

nx-cloud bot commented Jan 11, 2024 •

edited

Loading

codesandbox-ci bot commented Jan 11, 2024 •

edited

Loading

manudeli Jan 14, 2024 •

edited

Loading

manudeli commented Jan 14, 2024 •

edited

Loading

manudeli commented Jan 14, 2024 •

edited

Loading

codecov-commenter commented Jan 14, 2024 •

edited

Loading

manudeli commented Jan 14, 2024 •

edited

Loading

manudeli commented Jan 27, 2024 •

edited

Loading

manudeli commented Jan 27, 2024 •

edited

Loading