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): [require-types-exports] add rule#10554
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
feat(eslint-plugin): [require-types-exports] add rule#10554
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedDec 26, 2024 • 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 ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (88.94%) is below the target coverage (90.00%). You can increase the patch coverage or adjust thetarget coverage. Additional details and impacted files@@ Coverage Diff @@## main #10554 +/- ##==========================================+ Coverage 86.86% 86.88% +0.02%========================================== Files 445 446 +1 Lines 15455 15640 +185 Branches 4507 4545 +38 ==========================================+ Hits 13425 13589 +164- Misses 1675 1687 +12- Partials 355 364 +9
Flags with carried forward coverage won't be shown.Click here to find out more.
|
| @@ -31,9 +31,9 @@ interface Config { | |||
| }; | |||
| } | |||
| type Options = [Config]; | |||
| exporttype Options = [Config]; | |||
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.
Side note: Next rule has I lined the options and does not have Config as separate type. Maybe for consistency this type could be inlined too, if you are doing another PR just for these export changes :) Or if the separate Config type is preferred, maybe there could be some follow up
task for unifying these.
I don’t feel like this would matter if they would stay as internal types. But after they are exposed to “public”, it would look better if they had unified names. :)
rubiesonthesky 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.
I know this is a draft, but just few suggestions to not to export empty types.
Edit: I should have left these comments on the another PR which is just about the type
exports 😅
| type Options = []; | ||
| type MessageIds = 'useTopLevelQualifier'; | ||
| exporttype Options = []; | ||
| exporttype MessageIds = 'useTopLevelQualifier'; | ||
| export default createRule<Options, MessageIds>({ |
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.
| exportdefaultcreateRule<Options,MessageIds>({ | |
| exportdefaultcreateRule<[],MessageIds>({ |
| @@ -10,8 +10,8 @@ import { | |||
| NullThrowsReasons, | |||
| } from '../util'; | |||
| type Options = []; | |||
| type MessageIds = 'useTopLevelQualifier'; | |||
| export type Options = []; | |||
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.
| export type Options = []; |
(no longer needed)
| type Options = []; | ||
| type MessageIds = 'unaryMinus'; | ||
| exporttype Options = []; | ||
| exporttype MessageIds = 'unaryMinus'; | ||
| export default util.createRule<Options, MessageIds>({ |
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.
| exportdefaultutil.createRule<Options,MessageIds>({ | |
| exportdefaultutil.createRule<[],MessageIds>({ |
| @@ -3,8 +3,8 @@ import * as ts from 'typescript'; | |||
| import * as util from '../util'; | |||
| type Options = []; | |||
| type MessageIds = 'unaryMinus'; | |||
| export type Options = []; | |||
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.
| export type Options = []; |
I'm not going to have time for this anytime soon. Closing out so someone else can tackle this "fun" rule if they want. 🙂 |
PR Checklist
Overview
Continuation of@StyleShit's#8443 as indicated in#8443 (comment).
Co-authored-by:@StyleShit