Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat(eslint-plugin): [no-deprecated] add allow options#10585
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks for the PR,@yeonjuan! 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 onhttps://opencollective.com/typescript-eslint. |
netlifybot commentedJan 1, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site configuration. |
nx-cloudbot commentedJan 1, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
View yourCI Pipeline Execution ↗ for commit4ee9616.
☁️Nx Cloud last updated this comment at |
await apiV2(); | ||
function foo( | ||
element: HTMLFrameSetElement, // HTMLFrameSetElement is deprecated (not specified in the option) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
HTMLFrameSetElement
is deprecated, so when I check it in the playground, it shows a lint error. However, snapshot tests don't show lint errors, so maybe this is a bug in the snapshot test tsconfig configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hunch: that's from one including DOM types and the other not. I think switching this from alib
allow to afile
orpackage
allow would bypass that - and also be more representative of how most users would want to see this. But#9269 makes that hard because what we use in the playground isn't actually the same as docs and rule tests...
How about going with a@deprecated
global fromlib.es5.d.ts
, maybeescape
?http://localhost:3000/play#ts=5.7.2&fileType=.ts&code=KYZwxghgDsAUBEATgbwH-gSgFAFcB2okMCApAKIAsJAYgGw0DCmQA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1oBNFjpEZAIb5E3dGADa4bDkTRoHaJAA0M2VlnZIQ%2BPA4B3CdK1bNp2ZABmigLYTIjAEar1FnEyG3ED8kNKQbqYAvkHYALphobKREKHBQA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false
Examples of code for this rule with:```json{"allow": [{"from":"file","name":"escape" }]}```<Tabs><TabItemvalue="❌ Incorrect">```ts option='{"allow":[{"from":"lib","name":"escape"}]}'unescape('...');```</TabItem><TabItemvalue="✅ Correct">```ts option='{"allow":[{"from":"file","name":"escape"}]}'escape('...');```</TabItem></Tabs>
codecovbot commentedJan 1, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #10585 +/- ##==========================================+ Coverage 86.90% 86.95% +0.04%========================================== Files 446 446 Lines 15505 15512 +7 Branches 4517 4519 +2 ==========================================+ Hits 13475 13488 +13+ Misses 1675 1669 -6 Partials 355 355
Flags with carried forward coverage won't be shown.Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Functionality looks stellar! Just discussing docs changes now I think.
await apiV2(); | ||
function foo( | ||
element: HTMLFrameSetElement, // HTMLFrameSetElement is deprecated (not specified in the option) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hunch: that's from one including DOM types and the other not. I think switching this from alib
allow to afile
orpackage
allow would bypass that - and also be more representative of how most users would want to see this. But#9269 makes that hard because what we use in the playground isn't actually the same as docs and rule tests...
How about going with a@deprecated
global fromlib.es5.d.ts
, maybeescape
?http://localhost:3000/play#ts=5.7.2&fileType=.ts&code=KYZwxghgDsAUBEATgbwH-gSgFAFcB2okMCApAKIAsJAYgGw0DCmQA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1oBNFjpEZAIb5E3dGADa4bDkTRoHaJAA0M2VlnZIQ%2BPA4B3CdK1bNp2ZABmigLYTIjAEar1FnEyG3ED8kNKQbqYAvkHYALphobKREKHBQA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false
Examples of code for this rule with:```json{"allow": [{"from":"file","name":"escape" }]}```<Tabs><TabItemvalue="❌ Incorrect">```ts option='{"allow":[{"from":"lib","name":"escape"}]}'unescape('...');```</TabItem><TabItemvalue="✅ Correct">```ts option='{"allow":[{"from":"file","name":"escape"}]}'escape('...');```</TabItem></Tabs>
Uh oh!
There was an error while loading.Please reload this page.
@JoshuaKGoldberg Thanks! Fixed!3b45f65 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"allow": [ | ||
{ "from": "file", "name": "apiV1" }, | ||
{ "from": "lib", "name": "escape" }, | ||
{ "from": "package", "name": "Bar", "package": "bar-lib" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nit: since the package isn't referenced in the examples..
{ "from": "package", "name": "Bar", "package": "bar-lib" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oh. Thanks!
4ee9616
ronami left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The PR looks great, thank you 💯
Some thoughts:
Testing various cases, it seemsTypeOrValueSpecifier
is a bit more limited for the use-cases of this rule.
For example, I didn't manage to configure theallow
option not to report these:1,2. I haven't usedTypeOrValueSpecifier
extensively, so this might be an oversight on my end.
Yeah I suspect if this option gets a lot of usage, we'll get >=1 feature request to expand that format. 👍 |
6dda0a4
intotypescript-eslint:mainUh oh!
There was an error while loading.Please reload this page.
##### [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)##### 🚀 Features- **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))- **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))- **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))##### 🩹 Fixes- **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))- **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))##### ❤️ Thank You- Josh Goldberg ✨- Ronen Amiel- YeonJuan [@yeonjuan](https://github.com/yeonjuan)You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
##### [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)##### 🚀 Features- **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))- **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))- **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))##### 🩹 Fixes- **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))- **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))##### ❤️ Thank You- Josh Goldberg ✨- Ronen Amiel- YeonJuan [@yeonjuan](https://github.com/yeonjuan)You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
##### [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)##### 🚀 Features- **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))- **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))- **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))##### 🩹 Fixes- **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))- **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))##### ❤️ Thank You- Josh Goldberg ✨- Ronen Amiel- YeonJuan [@yeonjuan](https://github.com/yeonjuan)You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
##### [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)##### 🚀 Features- **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))- **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))- **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))##### 🩹 Fixes- **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))- **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))##### ❤️ Thank You- Josh Goldberg ✨- Ronen Amiel- YeonJuan [@yeonjuan](https://github.com/yeonjuan)You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
| datasource | package | from | to || ---------- | -------------------------------- | ------ | ------ || npm | @typescript-eslint/eslint-plugin | 8.19.1 | 8.20.0 || npm | @typescript-eslint/parser | 8.19.1 | 8.20.0 |## [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)##### 🚀 Features- **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))- **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))- **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))##### 🩹 Fixes- **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))- **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))##### ❤️ Thank You- Josh Goldberg ✨- Ronen Amiel- YeonJuan [@yeonjuan](https://github.com/yeonjuan)You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
| datasource | package | from | to || ---------- | -------------------------------- | ------ | ------ || npm | @typescript-eslint/eslint-plugin | 8.19.1 | 8.20.0 || npm | @typescript-eslint/parser | 8.19.1 | 8.20.0 |## [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)##### 🚀 Features- **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))- **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))- **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))##### 🩹 Fixes- **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))- **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))##### ❤️ Thank You- Josh Goldberg ✨- Ronen Amiel- YeonJuan [@yeonjuan](https://github.com/yeonjuan)You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
Overview
This PR addresses issue#9899.
Add an
allow
option of typeTypeOrValueSpecifier. The types/values specified in theallow
option are allowed to be used even if deprecated.