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

HenryBrown0 · 2023-09-04T19:20:10Z

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

…rst-prop-new-line`, `jsx-props-no-multi-spaces`, `propTypes`: use type args

codecov · 2023-09-04T19:24:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.66%. Comparing base (b4b7497) to head (f5bd822).
Report is 33 commits behind head on master.

❗ Current head f5bd822 differs from pull request most recent head fa6a122. Consider uploading reports for the commit fa6a122 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3629      +/-   ##
==========================================
+ Coverage   97.65%   97.66%   +0.01%     
==========================================
  Files         132      132              
  Lines        9379     9382       +3     
  Branches     3433     3445      +12     
==========================================
+ Hits         9159     9163       +4     
+ Misses        220      219       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ljharb

Seems good, altho there's an uncovered line.

lib/util/propTypes.js

lib/rules/boolean-prop-naming.js

henkkasoft · 2023-11-02T08:15:00Z

Hopefully this is fixed and merged soon.

ljharb · 2023-11-02T21:40:46Z

@HenryBrown0 any update on those smaller PRs?

HenryBrown0 · 2023-11-03T17:44:36Z

@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 · 2023-11-04T04:19:35Z

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 · 2023-11-27T19:53:37Z

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 · 2023-11-27T20:42:59Z

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@typescript-eslint/parser	5.62.0...6.15.0	None	`+6/-0`	33.7 MB	jameshenry

ljharb · 2023-11-27T21:09:56Z

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

osdiab · 2023-12-20T05:23:20Z

looking forward to this!

ljharb · 2023-12-20T07:09:35Z

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

HenryBrown0 · 2023-12-20T19:10:56Z

@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 · 2023-12-20T20:13:33Z

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

ljharb · 2023-12-20T20:33:54Z

@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).

HenryBrown0 · 2024-01-16T20:49:29Z

tests/helpers/parsers.js

@@ -37,7 +37,7 @@ const parsers = {
  BABEL_ESLINT: path.join(__dirname, NODE_MODULES, 'babel-eslint'),
  '@BABEL_ESLINT': path.join(__dirname, NODE_MODULES, '@babel/eslint-parser'),
  TYPESCRIPT_ESLINT: path.join(__dirname, NODE_MODULES, 'typescript-eslint-parser'),
-  '@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser'),
+  '@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser/dist'),


@typescript-eslint/parser removed the main field in v6, which pointed to dist/index.js. Not sure how tests were passing before, as this would have been wrong for all of v6.

exports['.'] would still work in node versions that support exports, which might be the only node versions that v6 supports?

v6 supports node versions ^16.0.0 || >=18.0.0, but even if I'm running this locally with node 18 it still fails to find the export

interesting, indeed node 18 should work - line 15 in https://unpkg.com/browse/@typescript-eslint/parser@6.19.0/package.json points there.

It works* locally when I switch it to importing the specific file:

Suggested change

'@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser/dist'),

'@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser/dist/index.js'),

*as in, other things break later 🙃 - https://github.com/jsx-eslint/eslint-plugin-react/pull/3629/files#r1580983575

lib/rules/jsx-first-prop-new-line.js

JoshuaKGoldberg · 2024-04-25T19:59:54Z

👋 @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 · 2024-04-26T12:18:20Z

👋 @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. #3629 (comment) was giving me issues before, maybe you have some ideas how this can be resolved?

JoshuaKGoldberg · 2024-04-26T12:47:36Z

package.json

Once https://github.com/jsx-eslint/eslint-plugin-react/pull/3629/files#r1454015312 is fixed, we get this lovely crash on any rule that uses @typescript-eslint/parser:

TypeError: import_typescript.default.isTokenKind is not a function at forEachToken (/Users/josh/repos/eslint-plugin-react/node_modules/ts-api-utils/lib/index.cjs:260:35) at Object.forEachComment (/Users/josh/repos/eslint-plugin-react/node_modules/ts-api-utils/lib/index.cjs:309:10) at convertComments (/Users/josh/repos/eslint-plugin-react/node_modules/@typescript-eslint/typescript-estree/dist/convert-comments.js:40:13) at astConverter (/Users/josh/repos/eslint-plugin-react/node_modules/@typescript-eslint/typescript-estree/dist/ast-converter.js:56:66) at parseAndGenerateServices (/Users/josh/repos/eslint-plugin-react/node_modules/@typescript-eslint/typescript-estree/dist/parser.js:170:66) at Object.parseForESLint (/Users/josh/repos/eslint-plugin-react/node_modules/@typescript-eslint/parser/dist/parser.js:93:80) at Object.parseForESLint (/Users/josh/repos/eslint-plugin-react/node_modules/eslint/lib/rule-tester/rule-tester.js:314:36) at parse (/Users/josh/repos/eslint-plugin-react/node_modules/eslint/lib/linter/linter.js:834:22) at Linter._verifyWithoutProcessors (/Users/josh/repos/eslint-plugin-react/node_modules/eslint/lib/linter/linter.js:1312:33) at Linter.verify (/Users/josh/repos/eslint-plugin-react/node_modules/eslint/lib/linter/linter.js:1454:57) at runRuleForItem (/Users/josh/repos/eslint-plugin-react/node_modules/eslint/lib/rule-tester/rule-tester.js:833:35) ...

This is because TypeScript is on an old version that doesn't support the default export:

eslint-plugin-react/package.json

Line 77 in 4467db5

"typescript": "^3.9.9",

I tried updating the devDependency to ^4.0.2 and then the error becamse:

TypeError: ts.getModifierFlags is not a function at Object.hasStaticModifierFlag (/Users/josh/repos/eslint-plugin-react/node_modules/typescript-estree/lib/node-utils.js:403:21) at convert (/Users/josh/repos/eslint-plugin-react/node_modules/typescript-estree/lib/convert.js:996:29) at convertChild (/Users/josh/repos/eslint-plugin-react/node_modules/typescript-estree/lib/convert.js:77:12) ...

...from the deprecated typescript-estree package. Which, along with typescript-eslint-parser, don't support the latest TypeScript versions. So they can't be used in environments that have typescript@4 (roughly - I haven't checked the exact range support).

eslint-plugin-react/package.json

Line 78 in 2e6b557

"typescript-eslint-parser": "^20.1.1"

When I set const tsOld = false; in parsers.js, then all tests pass with typescript@4 and @typescript-eslint/*@6.

@ljharb I can vaguely guess the right solution is to change around when the typescript-eslint-parser tests should be run, but I'm not clear on what the exact change should be. Is this something you can do?

I'll take a look - basically, we do some hijinks in our CI to install the right version combinations of things so it'll work.

[Fix] function-component-definition, boolean-prop-naming, `jsx-fi…

8004ff5

…rst-prop-new-line`, `jsx-props-no-multi-spaces`, `propTypes`: use type args

ljharb reviewed Sep 4, 2023

View reviewed changes

lib/util/propTypes.js Outdated Show resolved Hide resolved

ljharb reviewed Sep 6, 2023

View reviewed changes

lib/util/propTypes.js Outdated Show resolved Hide resolved

ljharb reviewed Sep 6, 2023

View reviewed changes

lib/rules/boolean-prop-naming.js Outdated Show resolved Hide resolved

HenryBrown0 mentioned this pull request Sep 24, 2023

[Refactor] extract type parameters to a variable #3634

Merged

ljharb mentioned this pull request Nov 4, 2023

False positive for react/jsx-no-leaked-render #3292

Open

ljharb force-pushed the fix/@typescript-eslint-v6-throws-deprecation-warnings branch from 0f45d97 to 586b162 Compare November 4, 2023 04:18

[Tests] add @typescript-eslint/parser v6

f5bd822

ljharb force-pushed the fix/@typescript-eslint-v6-throws-deprecation-warnings branch 3 times, most recently from 2929087 to f5bd822 Compare December 20, 2023 06:52

ljharb marked this pull request as ready for review December 20, 2023 06:52

ljharb approved these changes Dec 20, 2023

View reviewed changes

ljharb marked this pull request as draft December 20, 2023 07:09

fix: use @typescript-eslint/parser dist dir

4e8f750

HenryBrown0 commented Jan 16, 2024

View reviewed changes

ljharb mentioned this pull request Feb 10, 2024

[Bug]: typescript-eslint v6 deprecation warnings #3689

Closed

2 tasks

aslilac reviewed Mar 4, 2024

View reviewed changes

lib/rules/jsx-first-prop-new-line.js Outdated Show resolved Hide resolved

ljharb mentioned this pull request Apr 22, 2024

[Bug]: @typescript-eslint v6 throws deprecation warnings #3602

Open

2 tasks

JoshuaKGoldberg reviewed Apr 26, 2024

View reviewed changes

HenryBrown0 added 2 commits April 26, 2024 18:32

refactor: add getTypeArguments util

6c50ba1

refactor: tidy boolean prop naming type arg

fa6a122

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

HenryBrown0 commented Sep 4, 2023 •

edited

codecov bot commented Sep 4, 2023 •

edited

ljharb left a comment

henkkasoft commented Nov 2, 2023

ljharb commented Nov 2, 2023

HenryBrown0 commented Nov 3, 2023

ljharb commented Nov 4, 2023

HenryBrown0 commented Nov 27, 2023

socket-security bot commented Nov 27, 2023 •

edited

ljharb commented Nov 27, 2023

osdiab commented Dec 20, 2023

ljharb commented Dec 20, 2023

HenryBrown0 commented Dec 20, 2023

ljharb commented Dec 20, 2023

ljharb commented Dec 20, 2023

HenryBrown0 Jan 16, 2024

ljharb Jan 16, 2024

HenryBrown0 Jan 16, 2024

ljharb Jan 17, 2024

JoshuaKGoldberg Apr 26, 2024 •

edited

JoshuaKGoldberg commented Apr 25, 2024

HenryBrown0 commented Apr 26, 2024

JoshuaKGoldberg Apr 26, 2024

ljharb Apr 26, 2024

	'@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser/dist'),
	'@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser/dist/index.js'),

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

Are you sure you want to change the base?

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

Conversation

HenryBrown0 commented Sep 4, 2023 • edited

codecov bot commented Sep 4, 2023 • edited

Codecov Report

ljharb left a comment

Choose a reason for hiding this comment

henkkasoft commented Nov 2, 2023

ljharb commented Nov 2, 2023

HenryBrown0 commented Nov 3, 2023

ljharb commented Nov 4, 2023

HenryBrown0 commented Nov 27, 2023

socket-security bot commented Nov 27, 2023 • edited

ljharb commented Nov 27, 2023

osdiab commented Dec 20, 2023

ljharb commented Dec 20, 2023

HenryBrown0 commented Dec 20, 2023

ljharb commented Dec 20, 2023

ljharb commented Dec 20, 2023

HenryBrown0 Jan 16, 2024

Choose a reason for hiding this comment

ljharb Jan 16, 2024

Choose a reason for hiding this comment

HenryBrown0 Jan 16, 2024

Choose a reason for hiding this comment

ljharb Jan 17, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg Apr 26, 2024 • edited

Choose a reason for hiding this comment

JoshuaKGoldberg commented Apr 25, 2024

HenryBrown0 commented Apr 26, 2024

JoshuaKGoldberg Apr 26, 2024

Choose a reason for hiding this comment

ljharb Apr 26, 2024

Choose a reason for hiding this comment

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

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

HenryBrown0 commented Sep 4, 2023 •

edited

codecov bot commented Sep 4, 2023 •

edited

socket-security bot commented Nov 27, 2023 •

edited

JoshuaKGoldberg Apr 26, 2024 •

edited