Bump to typescript-eslint/utils 6.x

[stianjensen]

The 6.x branch of typescript-eslint requires at least eslint 7.x and typescript 4.2.4

It has also dropped support for node 12 and node 17.

[gund]

Hey, thanks for the PR and sorry for a delayed review!

Just left one comment, otherwise looks good!

[gund]

Looks good!

[gund]

Hmm, something wrong with the build on CI, do you know why it cannot fine @typescript-eslint/utils lib anymore?

[stianjensen]

Huh:
typescript-eslint/typescript-eslint#7284

[stianjensen]

Since no moduleResolution is specified in this project's tsconfig, it seems it defaults to 'node':
https://www.typescriptlang.org/tsconfig#moduleResolution

It should probably be updated to either node16 or nodenext.

[gund]

Omg, I hope this is not going to bring a full can of worms where all relative imports will have to end with .js.
Can you try updating tsconfig it and see if it works?

[stianjensen]

Tried to add that as a new commit now, so we'll see! 🤞

[gund]

Seems like the import still does not work in tests, maybe tsconfig for tests also needs similar fix?
Also there are a few type error in the build, could you take a look into those please?

[stianjensen]

Yeah, will at least need to also install @typescript-eslint/rule-tester now, as part of devDependencies, since it's extracted as its own package now.

[stianjensen]

Updated tsconfig for tests as well now + installed the missing new package. Let's see if that does the trick!

[gund]

Looks like tests are working now but there are still build errors and also a BC check for eslint v7 fails to install dependencies most likely because of new added dependencies which are not compatible with the older v7 major and should be removed before old versions are installed.

[stianjensen]

Yeah, the new rule-tester package seems to be making things complicated, by only supporting eslint >=8 and eslintrc >= 2.

But I guess we don't need to install version 4 of typescript-eslint packages to test eslint 7. The latest versions of typescript-eslint packages still support eslint 7. Looks from git history like that is leftover code all the way back initially creating those back-compat tests in 2021.

Some type errors I still need to figure out. services.program is typed as nullable now, but I'm not sure if that's relevant or if we can jsut safely use a new type that ensure it is always present (since type checking is marked as required for this rule):
https://typescript-eslint.io/blog/announcing-typescript-eslint-v6#type-checker-wrapper-apis

[gund]

What does this thing affect in docs btw?

[stianjensen]

Actually, I think we may want to delete this line – I couldn't find that the previous way of setting recommended: 'warn' makes sense for custom rules in typescript-eslint either, and it doesn't match the fact that it's set to 'error' in this package's recommended config:

eslint-plugin-deprecation/src/configs/recommended.ts

Line 4 in df4dc23

'deprecation/deprecation': 'error',

typescript-eslint themselves reworked their 'recommended' setup, into multiple new configs, which is why the types have changes, but the property does not seem to be documented.

[gund]

Hmm, okay then let's delete it altogether because it does not make any sense with recommended value anyway 😊

[gund]

Some type errors I still need to figure out. services.program is typed as nullable now, but I'm not sure if that's relevant or if we can jsut safely use a new type that ensure it is always present (since type checking is marked as required for this rule):

I guess it's marked as nullable in cases if typescript parser is not setup with eslint. Maybe we can catch this early in the rule and throw an error saying that typescript parser is required and link to this doc section?

[gund]

Seems like import { RuleTester } from '@typescript-eslint/rule-tester'; does not work in eslint v7 test with a really peculiar error message Cannot find module 'eslint/use-at-your-own-risk' from 'node_modules/@typescript-eslint/rule-tester/dist/utils/config-validator.js' 😂
Not sure if this new package actually supports eslint v7 but if not then we need to do something about it.
Since these are imports it's going to be very tricky to feature detect and import conditionally, unless we use dynamic imports.
We can also consider dropping eslint v7 support if that's what eslint ecosystem did in general and rid ourselves from this burden of backwards compat issues.
Thoughts?

UPDATE: Quick googling shows that some ppl also struggled with this when rules were upgraded to eslint 8 and they did something like this to support both eslint v7 and v8 (yeah ugly).

[stianjensen]

Seems like import { RuleTester } from '@typescript-eslint/rule-tester'; does not work in eslint v7

Ah, ofc. Yeah, the new package only supports eslint 8 (at least as declared by its peerDependencies).

Since the previous import of RuleTester is actually from @typescript-eslint/utils, a package which is part of this package's actual dependencies (not just devDependencies), it also seems like not a good idea to downgrade that package during the test, to get a reference to the old RuleTester. Unless it's somehow possibly to have it installed alongside @typescript-eslint/[email protected].

[stianjensen]

Still remain to figure out a way to detect whether type checking information is available, but pushed an experimental commit that attempts to install a parallel version of @typescript-eslint/utils only to use for getting a fallback RuleTester now.

[gund]

Oh well this already looks great, thanks for your efforts!
Just one more small thing from me 😊

[gund]

Hey it's working! Thanks for such a great effort to fix all the issues, I appreciate it!
I will merge it as a major change since we've dropped some major versions support.

[github-actions]

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀