-
-
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
Expose MatrixEvent's internal clearEvent as a function #1784
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.
🙌
(rr @t3chguy due to comment on the react-sdk side) |
*/ | ||
public getEffectiveEvent(): IEvent { | ||
// clearEvent doesn't have all the fields, so we'll copy what we can from this.event | ||
return Object.assign({}, this.event, this.clearEvent) as IEvent; |
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.
are you allergic to the nicer syntax :P?
return {
...this.event,
...this.clearEvent,
}
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.
"nicer" - I feel it doesn't communicate the order properly given JS objects are unordered, so there's no implicit guarantee that the spread operation will be applied correctly. The spec says it will, but that's a detail someone has to remember when writing/reading the code.
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
For element-hq/element-web#17615
For matrix-org/matrix-react-sdk#6371