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

feat: Disable typescript-eslint's array-type and consistent-type-definitions #1520

Closed
JoshuaKGoldberg opened this issue Jul 29, 2023 · 1 comment · Fixed by #1523
Closed
Labels
🔰 good first issue Good for newcomers 🌟 enhancement New feature or request 👨‍👦‍👦free for all Anyone is free to take on this issue and file a PR

Comments

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Jul 29, 2023

Is your feature request related to a problem? Please describe.

In #1476, create-t3-app started using "plugin:@typescript-eslint/stylistic-type-checked", which includes "plugin:@typescript-eslint/stylistic". Those opinionated stylistic config presets include a couple of rules that seem to be irritating users on Twitter:

Some folks like them enabled as-is. Others prefer the opposite. Either way, it looks like enabling them by default is turning people off of create-t3-app and/or linting and/or TypeScript in general. Although we know ESLint rules are straightforward you can turn off or reconfigure, it's still a bad first impression for others.

For reference, here are all the rules in the stylistic configs: https://typescript-eslint.io/rules/?supported-rules=stylistic

Describe the solution you'd like to see

These two rules seem to be the most consistently chafe-y from stylistic(-type-checked). Perhaps we should disable them in the .eslintrc.cjs template?

  extends: [
    "next/core-web-vitals",
    "plugin:@typescript-eslint/recommended-type-checked",
    "plugin:@typescript-eslint/stylistic-type-checked",
  ],
  rules: {
+   // These opinionated rules are enabled in stylistic-type-checked above.
+   // Feel free to reconfigure them to your own preference.
+   "@typescript-eslint/array-type": "off",
+   "@typescript-eslint/consistent-type-definitions": "off",
    // ...
  }

Describe alternate solutions

Not extending from stylistic-type-checked at all? This would mean less friction for users... but less benefit too. Most of the rules are pretty nice: no-inferrable-types, prefer-nullish-coalescing, prefer-optional-chain, ...

Additional information

I think this was a misplay on my end. We never resolved https://github.com/t3-oss/create-t3-app/pull/1476/files#r1234060608 over whether to include consistent-type-definitions, and I could have predicted that folks wouldn't like array-type. Sorry for causing user pain and tooling churn. 🙂

This is also making me wonder if we should turn those rules off in stylistic in a future major version of typescript-eslint... TBD.

@JoshuaKGoldberg JoshuaKGoldberg added the 🌟 enhancement New feature or request label Jul 29, 2023
@JacobMGEvans
Copy link
Contributor

I support this! @nexxeln @juliusmarminge it's you all's call though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔰 good first issue Good for newcomers 🌟 enhancement New feature or request 👨‍👦‍👦free for all Anyone is free to take on this issue and file a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants