-
-
Notifications
You must be signed in to change notification settings - Fork 590
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
Send/receive error details with widgets #4492
Conversation
869c438
to
6b47e22
Compare
@@ -147,6 +149,26 @@ export class RoomWidgetClient extends MatrixClient { | |||
) { | |||
super(opts); | |||
|
|||
const transportSend = this.widgetApi.transport.send.bind(this.widgetApi.transport); |
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 we need this bind?
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 is used as a way to preserve the original send
method so it can still be used by the function that replaces it.
Alternatively this could have been an arrow function, but that would require specifying a parameter list. Using bind gets around that.
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 like a really nasty pattern, this looks like somewhere a subclass makes far more sense rather than overwriting class methods, what's the rationale for not doing it in a more conventional way?
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.
It's because the send
methods don't belong to WidgetApi
, but to ITransport
, meaning that subclassing (or even a composition-based approach) would have to work harder to get to them.
Any non-bind
approach I can think of ends up being more complicated / requires much more copied code than this.
7a5961a
to
fbf6a7d
Compare
Depends on matrix-org/matrix-widget-api#100
Signed-off-by: Andrew Ferrazzutti [email protected]
Checklist
public
/exported
symbols have accurate TSDoc documentation.