-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Build: Migrate @storybook/channel-websocket to strict TS #22364
Build: Migrate @storybook/channel-websocket to strict TS #22364
Conversation
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! I left some feedback :)
@@ -20,9 +20,9 @@ interface CreateChannelArgs { | |||
} | |||
|
|||
export class WebsocketTransport { | |||
private socket: WebSocket; | |||
private socket?: WebSocket; |
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 would prefer inlining the connect function in the constructor, to show TS that it is always defined. As it is private, it is really only used in the constructor anyway.
if (this.handler) { | ||
this.handler(event); | ||
} |
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 think we expect a handler to be set here. Could you do a runtime assertion with tiny-invariant
?
invariant(this.handler, "WebsocketTransport handler should be set");
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 rolled bock one socket null check that is not neccesary anymore, LGTM now!
Closes #
What I did
Migrate @storybook/channel-websocket to strict-ts
How to test
Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:merged
orci:daily
GH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]