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
Refactor to improve types for declaration-*
rules
#5485
Conversation
This change removes `// @ts-nocheck` from the remaining `declaration-*` rules. Also, this adds 2 type parameters to the `StylelintRule` type to pass the type-check.
23a3c30
to
dddf856
Compare
secondaryOptions: Record<string, any>, | ||
export type StylelintRule<P = any, S = any> = (( | ||
primaryOption: P, | ||
secondaryOptions: Record<string, S>, |
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.
[note] P - Primary, S - Secondary
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.
Why do not use full words? 😄
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 follow the existing style, but is using full words common? (I'm open to change)
stylelint/types/stylelint/index.d.ts
Line 12 in dddf856
export type StylelintConfigRuleSettings<T, O extends Object> = |
stylelint/types/stylelint/index.d.ts
Line 51 in dddf856
type PropertyNamesOfType<T, U> = { |
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.
Let's follow the existing style. We can revisit the lot if it becomes an issue.
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.
Thanks as always, LGTM!
secondaryOptions: Record<string, any>, | ||
export type StylelintRule<P = any, S = any> = (( | ||
primaryOption: P, | ||
secondaryOptions: Record<string, S>, |
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.
Let's follow the existing style. We can revisit the lot if it becomes an issue.
Part of #4496
This change removes
// @ts-nocheck
from the remainingdeclaration-*
rules.Also, this adds 2 type parameters to the
StylelintRule
type to pass the type-check.