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

Should use standard '@typescript-eslint/' prefix for rule names, not 'ts/' #115

Closed
JoshuaKGoldberg opened this issue Jul 5, 2024 · 5 comments · Fixed by #128
Closed

Comments

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Jul 5, 2024

Describe the bug

The new ESLint flat config allows assigning any arbitrary prefix you want for rule names. The prefix used in the plugins object:

plugins: {
// @ts-expect-error
ts: tseslint.plugin,

...is then what the rules are referred to as in rules:

export const typescriptRules = {
/** Prefer Array<T> format */
'ts/array-type': ['error', { default: 'generic', readonly: 'generic' }],

Steps to reproduce

  1. import { tanstackConfig } from '@tanstack/config/eslint' in an eslint.config.js
  2. See that rules have names like ts/array-type, not @typescript-eslint/array-type

Expected behavior

The recommended preset configs from typescript-eslint use a '@typescript-eslint/' prefix, not 'ts/'. So if you try to, say, use both @tanstack/config/eslint and typescript-eslint in a config:

import { tanstackConfig } from '@tanstack/config/eslint'
import tseslint from 'typescript-eslint';

export default [
  ...tanstackConfig,
  { 
    rules: {
      'ts/no-explicit-any': 'error', // (just as an example)
    },
  },
  ...tseslint.configs.recommended,
]

...you'll get each rule running and reporting twice:

$ pnpm run test:eslint

...

   ✖  nx run @tanstack/form-core:test:eslint
      > @tanstack/[email protected] test:eslint /Users/josh/repos/tanstack-form/packages/form-core
      > eslint ./src ./tests
      
      
      /Users/josh/repos/tanstack-form/packages/form-core/src/FieldApi.ts
        418:13  error  Unexpected any. Specify a different type  ts/no-explicit-any
        418:13  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
      ...

Change request: can we switch the prefix to '@typescript-eslint/'?

How often does this bug happen?

Every time

Screenshots or Videos

No response

TanStack Config version

0.9.2

TypeScript version

No response

Additional context

I realize the ts/ prefix is much easier to read+write than the super verbose @typescript-eslint/. But we're stuck with @typescript-eslint/ in typescript-eslint because that's what everyone's been using for years.

We in typescript-eslint have previously surfaced feedback to ESLint core that prefix duplication is a known user pain: eslint/eslint#17766. But there isn't a way to lock down and/or deduplicate the prefixes (yet?).

Keeping to the standard prefix makes it easier to use standard shared configs: #114.

@TkDodo
Copy link

TkDodo commented Jul 15, 2024

@lachlancollins FYI

@lachlancollins
Copy link
Member

Hi @JoshuaKGoldberg , sorry I missed this! I'm surprised anyone is trying out this config outside of the TanStack repos! I was largely following antfu's config, and I see you've also submitted an issue there. I'm not too fussed by the prefix either way - ideally we would just use a config maintained by someone else, but there are some things in the antfu config that were too different from the existing TanStack eslint setups.

@TkDodo
Copy link

TkDodo commented Jul 15, 2024

I'm surprised anyone is trying out this config outside of the TanStack repos

For context, Josh reached out to me because he wants to help us upgrade to TypeScript eslint v8 beta, and since query uses the config from '@tanstack/config/eslint', I think we wound up here :)

@lachlancollins
Copy link
Member

For context, Josh reached out to me because he wants to help us upgrade to TypeScript eslint v8 beta, and since query uses the config from '@tanstack/config/eslint', I think we wound up here :)

Ahhh makes sense! It looks like ESLint v8.57.0 is still supported by the typescript-eslint v8 beta, so we could upgrade that here without too much difficulty. @JoshuaKGoldberg I'm very happy if you want to submit a PR to change the prefix or anything else :)

@JoshuaKGoldberg
Copy link
Contributor Author

and because query uses the config from '@tanstack/config/eslint'

Yup, exactly!

submit a PR to change the prefix or anything else :)

Awesome, thanks! Will do now. 🚀

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

Successfully merging a pull request may close this issue.

3 participants