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.

Sign up for GitHub

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

Jump to bottom

feat(ast-spec): remove deprecated type params #8933

Open
wants to merge 20 commits into
base: v8
Choose a base branch
from
Open

feat(ast-spec): remove deprecated type params #8933

wants to merge 20 commits into from

Conversation

abrahamguo
Copy link
Contributor

@abrahamguo abrahamguo commented Apr 15, 2024 •
edited by JoshuaKGoldberg

BREAKING CHANGE:
Removes properties from the AST.

PR Checklist

Overview

Remove @deprecated typeParameters properties that were deprecated in #6274

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @abrahamguo!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

@netlify Netlify
Copy link

netlify bot commented Apr 15, 2024 •
edited

✅ Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 342cb68
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/662a436d6035470008ab75b2
😎 Deploy Preview https://deploy-preview-8933--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 98 (🟢 up 5 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 98 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Apr 15, 2024 •
edited

☁️ Nx Cloud Report

CI is running/has finished running commands for commit da3fc57. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 31 targets

Sent with 💌 from NxCloud.

@abrahamguo abrahamguo changed the title enhancement(ast-spect): remove deprecated type params feat(ast-spec): remove deprecated type params Apr 15, 2024
@abrahamguo abrahamguo changed the base branch from main to v8 April 16, 2024 12:03
@abrahamguo abrahamguo changed the base branch from v8 to main April 16, 2024 12:03
@auvred auvred added the breaking change This change will require a new major version to be released label Apr 16, 2024
JoshuaKGoldberg
JoshuaKGoldberg requested changes Apr 22, 2024
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

(waiting on test failures)

@JoshuaKGoldberg JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Apr 22, 2024
abrahamguo and others added 9 commits April 22, 2024 21:01
@abrahamguo
remove deprecated aliases
ac7f0e2
@abrahamguo
remove outdated tests
2ebf76c
@abrahamguo
turn off prop types
12ace23
@auvred
fix(utils): export ESLint backwards-compat functions (typescript-esli…
1763889 
…nt#8976)
@developer-bandi
test(type-utils): add unit tests for requiresQuoting (typescript-esli…
ba1eb20 
…nt#8691)

* test: add testCase

* test: fix test case result

* docs: add comment in testcase and function
@yeonjuan
fix(eslint-plugin): [no-unsafe-argument] handle tagged templates (typ…
b0f7aa4 
…escript-eslint#8746)

* feat(eslint-plugin): [no-unsafe-argument] handle  tagged templates

* add istanbul comment

* add test cases

* fix error loc

* fix: add null throws
@abrahamguo
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
8cfaa82 
…emove-type-parameters
@jsfm01 @up201304504 @JoshuaKGoldberg
fix(eslint-plugin): [prefer-optional-chain] suggests optional chainin…
4bed24d 
…g during strict null equality check (typescript-eslint#8717)

* fix [prefer-optional-chain] suggests optional chaining during strict null equality

* fix [prefer-optional-chain] suggests optional chaining during strict null equality

* Fix lint

* Add prefer-optional-chain test

* Fix lint

* rebase and fix prefer optional chain on strict null equality

* fix prefer optional chain on strict null equality

* fix lint

* tests: code review suggestion

* Update packages/eslint-plugin/src/rules/prefer-optional-chain-utils/analyzeChain.ts

---------

Co-authored-by: Marta Cardoso <up201304504@up.pt>
Co-authored-by: Marta Cardoso <145560260+up201304504@users.noreply.github.com>
Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
@abrahamguo
Merge branch 'main' of github.com:abrahamguo/typescript-eslint into r…
342cb68 
…emove-type-parameters
@abrahamguo abrahamguo changed the base branch from main to v8 April 25, 2024 11:50
@github-actions github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Apr 25, 2024
JoshuaKGoldberg
JoshuaKGoldberg approved these changes Apr 25, 2024
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment •
edited

Choose a reason for hiding this comment

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

LGTM, thanks! 🙌 Very excited to see most of our @deprecated AST spec properties gone.

Since this touches sensitive code, I'll leave it with 1 approval for another member of @typescript-eslint/triage-team to take a look. I don't trust myself. 🙂

Old timey Smurf cartoon showing a smurf sweeping a floor

eslint.config.mjs
Comment on lines +516 to +518
/* This rule
(1) causes false reports because it reads from `typeParameters` rather than `typeArguments`, and
(2) is unnecessary in TypeScript components */
Copy link
Member

Choose a reason for hiding this comment

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

For (1), if it's a transient problem, we'd want to link to the upstream issue (jsx-eslint/eslint-plugin-react#3602). Vague comments tend to get out of date and don't help new readers understand the issue well.

For (2), yup, that's going to last beyond (1) - so I'd say it makes sense to disable the rule here always anyway.

Suggested change
/* This rule
(1) causes false reports because it reads from `typeParameters` rather than `typeArguments`, and
(2) is unnecessary in TypeScript components */

@JoshuaKGoldberg JoshuaKGoldberg added the 1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready label Apr 25, 2024
@JoshuaKGoldberg JoshuaKGoldberg added this to the 8.0.0 milestone Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

Assignees
No one assigned
Labels
1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready breaking change This change will require a new major version to be released
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

Enhancement: Remove AST properties deprecated in v6
6 participants
@abrahamguo @JoshuaKGoldberg @auvred @developer-bandi @yeonjuan @jsfm01