-
-
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
Marionette: migrate to TS #13448
Marionette: migrate to TS #13448
Conversation
@@ -8,10 +8,10 @@ const allMarionetteViewConstructors = [ | |||
]; | |||
const viewConstructorsSupportedByMarionette = allMarionetteViewConstructors | |||
.filter((constructorName) => constructorName in Marionette) | |||
.map((constructorName) => Marionette[constructorName]); | |||
.map((constructorName) => (Marionette as any)[constructorName]); |
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 tried to find the correct type but it's too 🤯 for a non-marionette user.
export const storiesOf = (...args: any) => | ||
clientApi.storiesOf(...args).addParameters({ framework }); | ||
export const load = (...args: any) => coreLoad(...args, framework); |
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.
Will be properly type as soon as I will rework some @storybook/core
types
export function webpack(config: Configuration) { | ||
return 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.
Not sure we need this but I didn't want to make changes to production code other than a simple TS migration.
2798d1e
to
a3d00fc
Compare
a3d00fc
to
2150e27
Compare
2150e27
to
3f67f70
Compare
I think we are good to go with this one @ndelangen @shilman, not sure the diffs in chromatic are relevant/related to this PR. |
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.
LGTM
IMHO take time to add robust typescript is interesting if users (+framework) are also adepts. if they are users of "js" this is not (or less) necessary 🙊
Issue: Part of #5030
What I did
Fastly migrate
@storybook/marionette
to TS as I want to close the big TS Migration Issue before moving to improve types of@storybook/core
.How to test