chore: brought in typescript-eslint@v8 with stylistic preset #10911

JoshuaKGoldberg · 2024-07-04T20:03:06Z

This applies two internal tooling updates:

Bumps typescript-eslint to its beta v8: https://typescript-eslint.io/blog/announcing-typescript-eslint-v8-beta
Enables typescript-eslint's shared stylistic config for internal code (not eslint-config)

Any changes (will be) commented inline. This also doesn't tackle three good followups:

Rules with many existing reports I kept disabled under a TODO: comment in .eslintrc.js
- A reasonable next step for the project would be to look through those and decide which should be fixed up.
This doesn't enable typed linting internally
- Would definitely recommend next!
Onboarding to ESLint's flat config
- Blocked on [Feature]: ESLint flat config support for @redwoodjs/eslint-config #10897

There should be no user-facing functional changes. It should just be cleanups.

⚠️ This isn't ready for merge, as it uses rc-v8 versions of typescript-eslint. Just posting as a reference prior to v8 going stable.

Once v8 is stable, I can update this to use the released version.
Alternately, if you'd like these changes sooner, I can make a version of this that doesn't update to rc-v8 - it'll just have slightly fewer changes...

This PR came out of a casual conversation with @Tobbe. I'm happy to file an issue / make a more formal internal tooling request if you'd prefer. ❤️

.eslintrc.js

packages/adapters/fastify/web/src/resolveOptions.ts

packages/auth-providers/dbAuth/setup/src/__tests__/setup.test.ts

packages/babel-config/src/plugins/babel-plugin-redwood-directory-named-import.ts

packages/eslint-config/shared.js

packages/eslint-plugin/src/index.ts

packages/structure/src/model/RWTOML.ts

packages/structure/src/model/RWRoute.ts

packages/structure/src/model/RWTOML.ts

packages/eslint-config/shared.js

Tobbe · 2024-07-08T12:34:01Z

@JoshuaKGoldberg I like the fixes/changes to the source code that this has prompted!

Would you be up for moving those out into a separate PR that we can merge right away? This PR currently touches so many files it's highly likely a bunch of them will change while we wait for v8 to reach GA. So to not have to worry about so many potential merge conflicts I figured we could get those changes merged asap 🙂

JoshuaKGoldberg · 2024-07-08T12:42:31Z

You got it! #10924

Tobbe · 2024-07-08T13:28:41Z

From 55 to 8 changed files after #10924 got merged! Nice!

Tobbe · 2024-07-08T14:30:43Z

packages/eslint-config/shared.js

-        // 'recommended' rules we alter
-        '@typescript-eslint/no-explicit-any': 'warn',
+        // TODO: look into enabling these eventually
+        '@typescript-eslint/no-empty-function': 'off',


I tried turning this on, but didn't see any errors being reported.
Same for the rule below.

Am I doing something wrong? Or was this to limit the effect of this PR on user's apps?

Oh shoot sorry @Tobbe, I missed this comment. It was to limit the effect of this PR on user's apps. I remember that some of these had >0 failures but didn't pay attention to which.

Josh-Walker-GM · 2024-08-13T18:06:55Z

@JoshuaKGoldberg I pushed through the renovate update to get us to v8. Do you want me to get this PR back to a non conflicted state and look to get this in?

JoshuaKGoldberg · 2024-08-13T18:20:08Z

Sure! Let me know if you run out of time, I can help too. 🙂

…listic-and-rc-v8

Josh-Walker-GM · 2024-08-13T19:03:05Z

Last time I was working on linting rules I hacked together a quick script which helps me understand the impact on a users project. A summary of what that has told me is:

jsx-a11y/no-noninteractive-element-to-interactive-role is different - looks like it's default internal config is just updated - fine for us
@typescript-eslint/explicit-function-return-type - used to be turned off, now it's gone
@typescript-eslint/no-empty-interface - used to be turned off, now it's gone
@typescript-eslint/explicit-module-boundary-types - used be turned off, now it's gone
@typescript-eslint/ban-types - used to be 'warn', now it's gone. This has been replaces with a set of smaller more specific rules.
no-empty-function - used be turned off, not it's gone. The ts-eslint flavour is still there and turned off still.
camelcase - used to be turned off, now it's 'warn'
@typescript-eslint/camelcase - used to be turned off, now it's gone
no-use-before-define - used to be turned off, now it's gone
@typescript-eslint/no-use-before-define - used to be turned off, now it's gone
@typescript-eslint/prefer-namespace-keyword - used to be turned off, now it's 'error'
unicode-bom - used to be turned off, now it's gone
@typescript-eslint/adjacent-overload-signatures - used to be 'error', now it's gone
@typescript-eslint/no-explicit-any - used be 'warn', now 'error'
@typescript-eslint/no-inferrable-types - used to be 'error', now it's gone
no-loss-of-precision - used be 'off', now 'error'
@typescript-eslint/no-loss-of-precision - used be 'error', now gone
@typescript-eslint/no-non-null-assertion - used be 'warn', now gone
valid-typeof - used be either 'error' or 'off', now always 'error'
no-unused-expressions - used be always 'error', now either 'error' or 'off'
@typescript-eslint/prefer-function-type - newly added as 'off'
@typescript-eslint/no-require-imports - newly added as 'off'
@typescript-eslint/no-empty-object-type - newly added as 'off'
unicorn/template-indent - newly added as 'off'
@typescript-eslint/no-duplicate-enum-values - newly added as 'error'
@typescript-eslint/no-unsafe-declaration-merging - newly added as 'error'
@typescript-eslint/no-unsafe-function-type - newly added as 'error'
@typescript-eslint/no-unused-expressions - newly added as 'error'
@typescript-eslint/no-wrapper-object-types - newly added as 'error'
no-new-native-nonconstructor - newly added as 'off'

Josh-Walker-GM · 2024-08-13T20:56:38Z

I'm happy with those user facing changes to go out in our current v8 major release. @JoshuaKGoldberg Are you happy with this PR and I'll merge?

JoshuaKGoldberg · 2024-08-13T22:07:43Z

No blockers from me!

I'll note that jsx-a11y/no-noninteractive-element-to-interactive-role isn't from typescript-eslint, just from a dependency.

Josh-Walker-GM · 2024-08-13T22:09:34Z

Awesome I'll look over this again and then merge it!

The little diff script just dumps all the changes in applied linting rules it finds. Thanks for confirming that though!

Tobbe

LGTM

…pload-link * 'main' of github.com:redwoodjs/redwood: chore(linting): Remove/fix references to non-existant files (redwoodjs#11245) chore(rsa): Use swc for parsing server actions (redwoodjs#11243) chore(lint): Remove override for 'unused-imports/no-unused-imports' (redwoodjs#11244) chore(linting): Separate out framework and user linting config (redwoodjs#11242) fix: Update default tsconfig options (target, module and moduleResolution) (redwoodjs#11170) chore(fixture): Update tailwind dep (redwoodjs#11241) chore(deps): bump fast-xml-parser from 4.4.0 to 4.4.1 (redwoodjs#11239) chore(rsc): Switch last remaining transform-server test to inline snapshot (redwoodjs#11240) chore: brought in typescript-eslint@v8 with stylistic preset (redwoodjs#10911) chore(deps): bump axios from 1.7.3 to 1.7.4 (redwoodjs#11237) docs(serverConfig): Remove server config option from TOML (redwoodjs#11236) fix(deps): update typescript-eslint monorepo to v8 (major) (redwoodjs#11235)

chore: brought in typescript-eslint@v8 with stylistic preset

68fa68b