TanStack / config

Configuration and tools for publishing and maintaining high-quality JavaScript packages
https://tanstack.com/config
MIT License
202 stars 11 forks source link

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

Closed JoshuaKGoldberg closed 4 months ago

JoshuaKGoldberg commented 4 months ago

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:

https://github.com/TanStack/config/blob/2935da96712b917efd39f06d440998a5a8c49e1b/packages/config/src/eslint/index.js#L44-L46

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

https://github.com/TanStack/config/blob/2935da96712b917efd39f06d440998a5a8c49e1b/packages/config/src/eslint/typescript.js#L4-L6

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/form-core@0.25.2 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: https://github.com/eslint/eslint/discussions/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: https://github.com/TanStack/config/discussions/114.

TkDodo commented 4 months ago

@lachlancollins FYI

lachlancollins commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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. 🚀