[Fix] `@typescript-eslint` v6 use typeArguments with fallback to typeParameters

[HenryBrown0]

Follows the guide from @typescript-eslint to upgrade to v6, by adding a small utility function getTypeArguments which returns the typeArguments or falls back to typeParameters.

Closes #3602

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 61.36364% with 17 lines in your changes missing coverage. Please review.

Project coverage is 94.42%. Comparing base (597553d) to head (e3ee2b0).

Files	Patch %	Lines
lib/util/propTypes.js	30.00%	14 Missing :warning:
lib/rules/jsx-props-no-multi-spaces.js	60.00%	2 Missing :warning:
lib/util/props.js	75.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3629      +/-   ##
==========================================
- Coverage   97.62%   94.42%   -3.21%     
==========================================
  Files         132      132              
  Lines        9692     9703      +11     
  Branches     3520     3522       +2     
==========================================
- Hits         9462     9162     -300     
- Misses        230      541     +311     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[henkkasoft]

Hopefully this is fixed and merged soon.

[ljharb]

@HenryBrown0 any update on those smaller PRs?

[HenryBrown0]

@HenryBrown0 any update on those smaller PRs?

Sorry I got side tracked, I'll try getting some tests written in the next few days

[ljharb]

Can we add some tests that would have failed without these typeArguments checks? That may require adding tests that run on typescript-eslint v6.

[HenryBrown0]

Can we add some tests that would have failed without these typeArguments checks? That may require adding tests that run on typescript-eslint v6.

typeParameters has been marked as @deprecated rather than removed, I'm not sure how writing tests will work here. I've added v6 to ci. Hopefully this is good to go?

[socket-security[bot]]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@typescript-eslint/[email protected]	Transitive: environment, eval, filesystem, shell, unsafe	+110	12.6 MB	jameshenry

🚮 Removed packages: npm/@typescript-eslint/[email protected]

View full report↗︎

[ljharb]

Fair, I'll take that (and rebase this) assuming tests pass :-) thanks!

[osdiab]

looking forward to this!

[ljharb]

@HenryBrown0 unfortunately a number of tests are failing. any idea why?

[HenryBrown0]

@HenryBrown0 unfortunately a number of tests are failing. any idea why?

Unit tests are failing on master for me, has a dependency changed?

It appears only minor version of Node.js 6 are failing, e.g. 18.6.8 and 19.6.8. I suspect this is a red herring as the matrix build reports using different node versions;

• 18.5.9 uses 18.19.0 and passes
• 18.6.8 uses 18.19.0 and fails Not sure why this is happening

[ljharb]

Thanks, in that case I'll check out master and report back.

[ljharb]

@HenryBrown0 tests seem to be passing on master https://github.com/jsx-eslint/eslint-plugin-react/actions (i checked on my fork before pushing to this repo, so i expect these to pass).

[JoshuaKGoldberg]

👋 @HenryBrown0 do you still have time to work on this? We're at v7 in typescript-eslint (https://typescript-eslint.io/blog/announcing-typescript-eslint-v7) and working on v8 - which will remove the old properties altogether.

Even if there are still some cases that log for deprecated property access (https://github.com/jsx-eslint/eslint-plugin-react/pull/3629/files#r1511579466) I personally would very much like to see this PR merged. That way the v8 branch of typescript-eslint can use the impacted rules. Want any help? ❤️

[HenryBrown0]

👋 @HenryBrown0 do you still have time to work on this? We're at v7 in typescript-eslint (https://typescript-eslint.io/blog/announcing-typescript-eslint-v7) and working on v8 - which will remove the old properties altogether.

Even if there are still some cases that log for deprecated property access (https://github.com/jsx-eslint/eslint-plugin-react/pull/3629/files#r1511579466) I personally would very much like to see this PR merged. That way the v8 branch of typescript-eslint can use the impacted rules. Want any help? ❤️

I'm going to look at this again this evening. https://github.com/jsx-eslint/eslint-plugin-react/pull/3629#discussion_r1454015312 was giving me issues before, maybe you have some ideas how this can be resolved?

[pzimmermaninmo]

@HenryBrown0 I found something very similar and wanted to get your thoughts

I tested this PR with my codebase and it in fact fixed the original error DeprecationWarning: The 'typeParameters' property is deprecated on CallExpression nodes. Use 'typeArguments' instead.

However, I am now getting a very similar error where superTypeParameters is deprecated and superTypeArguments should be used instead. Have you seen similar? Does it sound like something that should be fixed here too?

[HenryBrown0]

@HenryBrown0 I found something very similar and wanted to get your thoughts

I tested this PR with my codebase and it in fact fixed the original error DeprecationWarning: The 'typeParameters' property is deprecated on CallExpression nodes. Use 'typeArguments' instead.

However, I am now getting a very similar error where superTypeParameters is deprecated and superTypeArguments should be used instead. Have you seen similar? Does it sound like something that should be fixed here too?

This is a question for @ljharb, if it's a deprecated warning I imagine the preference would be to get this in first and a pr after for superTypeParameters

[pzimmermaninmo]

@HenryBrown0 I found something very similar and wanted to get your thoughts I tested this PR with my codebase and it in fact fixed the original error DeprecationWarning: The 'typeParameters' property is deprecated on CallExpression nodes. Use 'typeArguments' instead. However, I am now getting a very similar error where superTypeParameters is deprecated and superTypeArguments should be used instead. Have you seen similar? Does it sound like something that should be fixed here too?

This is a question for @ljharb, if it's a deprecated warning I imagine the preference would be to get this in first and a pr after for superTypeParameters

Definitely a bit of scope creep for sure. I just figured I'd mention especially since its coming from the same propTypes.js file.

[ljharb]

It'd be fine to include it in this PR, or a follow-on; but since this one still has lots of failing tests we probably shouldn't increase complexity yet :-)

[hampustagerud]

@ljharb I read in https://github.com/jsx-eslint/eslint-plugin-react/issues/3602#issuecomment-2070961147 that you're open to pulling in additional changes to this PR, rather than opening another PR, so I'm here to submit my branch which updates the workflows to pass the tests. In order to get the workflows to run properly I had to disable legacy peer dependency installation for jobs running v6+ of the parser, since it requires it's peer dependencies (typescript in this case) to be installed correctly.

I have also added v7 and v8 to the test matrix (you can check the runs for more details), since that is kind of blocking our Eslint 9 upgrade right now 🙂 https://github.com/jsx-eslint/eslint-plugin-react/pull/3629#issuecomment-2116022056 turned out to be an issue, but only very minor changes required in the code 👍