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

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

Conversation

HenryBrown0
Copy link
Contributor

@HenryBrown0 HenryBrown0 commented Sep 4, 2023

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
Copy link

codecov bot commented Sep 4, 2023

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 ⚠️
lib/rules/jsx-props-no-multi-spaces.js 60.00% 2 Missing ⚠️
lib/util/props.js 75.00% 1 Missing ⚠️
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     

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

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good, altho there's an uncovered line.

lib/util/propTypes.js Outdated Show resolved Hide resolved
lib/util/propTypes.js Outdated Show resolved Hide resolved
@henkkasoft
Copy link

Hopefully this is fixed and merged soon.

@ljharb
Copy link
Member

ljharb commented Nov 2, 2023

@HenryBrown0 any update on those smaller PRs?

@HenryBrown0
Copy link
Contributor Author

@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 ljharb force-pushed the fix/@typescript-eslint-v6-throws-deprecation-warnings branch from 0f45d97 to 586b162 Compare November 4, 2023 04:18
@ljharb
Copy link
Member

ljharb commented Nov 4, 2023

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

…rst-prop-new-line`, `jsx-props-no-multi-spaces`, `propTypes`: use type args
@HenryBrown0
Copy link
Contributor Author

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?

Copy link

socket-security bot commented Nov 27, 2023

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

Package New capabilities Transitives Size Publisher
npm/@typescript-eslint/[email protected] Transitive: environment, filesystem +26 1.97 MB jameshenry

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

View full report↗︎

@ljharb
Copy link
Member

ljharb commented Nov 27, 2023

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

@osdiab
Copy link

osdiab commented Dec 20, 2023

looking forward to this!

@ljharb 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 ljharb marked this pull request as ready for review December 20, 2023 06:52
@ljharb ljharb marked this pull request as draft December 20, 2023 07:09
@ljharb
Copy link
Member

ljharb commented Dec 20, 2023

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

@HenryBrown0
Copy link
Contributor Author

@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
Copy link
Member

ljharb commented Dec 20, 2023

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

@ljharb
Copy link
Member

ljharb commented Dec 20, 2023

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

 - only install peer dependencies with legacy mode when testing @typescript-eslint/parser < v6
tests/helpers/parsers.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: @typescript-eslint v6 throws deprecation warnings
8 participants