-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Fix type of function makeDecorator
from lib/addons
#5534
Conversation
@@ -67,7 +67,7 @@ export const withCssResources = makeDecorator({ | |||
skipIfNoParametersOrOptions: true, | |||
allowDeprecatedUsage: false, | |||
|
|||
wrapper: (getStory: any, context: any, { options, parameters }: any) => { | |||
wrapper: (getStory, context, { options, parameters }) => { |
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.
This seems unrelated to your PR, but good catch anyways 👍
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.
Just found it when I was looking for makeDecorator
usage, so I cleaned it ;)
name, | ||
parameterName, | ||
wrapper, | ||
skipIfNoParametersOrOptions = false, | ||
allowDeprecatedUsage = false, | ||
}: MakeDecoratorOptions) => { | ||
}: MakeDecoratorOptions): MakeDecoratorResult => { |
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.
Good catch 👍
@@ -31,13 +31,13 @@ interface MakeDecoratorOptions { | |||
wrapper: StoryWrapper; | |||
} | |||
|
|||
export const makeDecorator: MakeDecoratorResult = ({ | |||
export const makeDecorator = ({ | |||
name, | |||
parameterName, | |||
wrapper, | |||
skipIfNoParametersOrOptions = false, | |||
allowDeprecatedUsage = false, |
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.
Both skipIfNoParametersOrOptions
and allowDeprecatedUsage
have default values set here, so you shouldn't have to set those in the tests explicitly. But I leave it up to you.
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.
Ok, I understand why I had to explicitly set them in test, just because they are not declared as optional in MakeDecoratorOptions
. I will make them optional and revert my changes on tests.
`MakeDecoratorResult` is type of object returned by `makeDecorator` and not `makeDecorator` itself. This fix also helps a lot in addons TS migration because correct types are now inferred. Make `allowDeprecatedUsage` and `skipIfNoParametersOrOptions` as they have default value in `makeDecorator`
Codecov Report
@@ Coverage Diff @@
## next #5534 +/- ##
=====================================
Coverage 33.1% 33.1%
=====================================
Files 638 638
Lines 9264 9264
Branches 1340 1318 -22
=====================================
Hits 3067 3067
Misses 5571 5571
Partials 626 626
Continue to review full report at Codecov.
|
What I did
Fix type of function
makeDecorator
fromlib/addons
.MakeDecoratorResult
is type of object returned bymakeDecorator
and notmakeDecorator
itself.This fix also helps a lot in addons TS migration because correct types are now inferred 😃.
Also remove some
any
typings linked tomakeDecorator
inaddon/cssresources