Skip to content

feat(eslint-plugin): deprecate prefer-ts-expect-error in favor of ban-ts-comment #9081

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
Merged
merged 2 commits into from
May 26, 2024
Merged

feat(eslint-plugin): deprecate prefer-ts-expect-error in favor of ban-ts-comment #9081

merged 2 commits into from
May 26, 2024

Conversation

auvred
Copy link
Member

@auvred auvred commented May 12, 2024

PR Checklist

Overview

prefer-ts-expect-error is just a subset of ban-ts-comment because ban-ts-comment also suggests replacing @ts-ignore with @ts-expect-error

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @auvred!

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 May 12, 2024 •
edited
Loading

✅ Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 591fefa
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/6641c3b1424c0300086d9786
😎 Deploy Preview https://deploy-preview-9081--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: 99 (🟢 up 3 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 May 12, 2024 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 591fefa. 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.

@kirkwaiblinger
Copy link
Member

kirkwaiblinger commented May 13, 2024 •
edited
Loading

It looks like the docs website doesn't have any accompanying changes to indicate the deprecation other than it showing in the deprecated column of the table... Consider adding a doc warning like the following?

typescript-eslint/packages/eslint-plugin/docs/rules/no-throw-literal.mdx

Lines 17 to 23 in 7ce6acd

:::warning
This rule is being renamed to [`only-throw-error`](./only-throw-error.mdx).
When it was first created, it only prevented literals from being thrown (hence the name), but it has now been expanded to only allow expressions which have a possibility of being an `Error` object.
With the `allowThrowingAny` and `allowThrowingUnknown` options, it can be configured to only allow throwing values which are guaranteed to be an instance of `Error`.
The current name `no-throw-literal` will be removed in a future major version of typescript-eslint.
:::
(though note that that's also being tweaked a bit in #8821 (comment))

@auvred
Copy link
Member Author

auvred commented May 13, 2024

Oops, I kept in mind to mention deprecation in the docs, but it slipped my mind for some reason 😄 Thanks!

It might be worth mentioning this in Rule Deprecation docs:

typescript-eslint/docs/maintenance/issues/Rule_Deprecations.mdx

Lines 28 to 34 in 7ce6acd

## Timeline
1. In any minor/patch version, add [rule `meta` properties](https://eslint.org/docs/latest/developer-guide/working-with-rules#rule-basics):
- `deprecated: true`
- `replacedBy`, if applicable
2. In the next major version, you may delete the rule
- If the rule is relatively popular with users, consider leaving a documentation page as a tombstone pointing to the new relevant rule or docs (see [`camelcase`](/rules/camelcase/) as an example)

JoshuaKGoldberg
JoshuaKGoldberg approved these changes May 26, 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.

LGTM, thanks!

+1 if someone wants to file a followup issue / send a followup PR about adding this to the maintenance docs. 😄

@JoshuaKGoldberg JoshuaKGoldberg merged commit b230385 into typescript-eslint:main May 26, 2024
57 of 58 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rules: Deprecate prefer-ts-expect-error in favor of ban-ts-comment
3 participants
@auvred @kirkwaiblinger @JoshuaKGoldberg