Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix integration manager not updating when set #3510

Merged
merged 1 commit into from
Oct 3, 2019
Merged

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Oct 2, 2019

setUserWidget was modifying the content of the old event itself,
so when waitForUserWidget() checked the content to see if it was
there yet, it was, but because the echo hadn't come back, the
IntegrationManager hadn't rebuilt its list.

In other news, its terrifying that we can just accidentally modify
the content of an event in the store. I'm going to make a js-sdk
PR that freezes the content and see what breaks...

Fixes element-hq/element-web#10977

setUserWidget was modifying the content of the old event itself,
so when `waitForUserWidget()` checked the content to see if it was
there yet, it was, but because the echo hadn't come back, the
IntegrationManager hadn't rebuilt its list.

In other news, its terrifying that we can just accidentally modify
the content of an event in the store. I'm going to make a js-sdk
PR that freezes the content and see what breaks...

Fixes element-hq/element-web#10977
@dbkr dbkr requested a review from a team October 2, 2019 14:31
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohgod.gif

const userWidgets = WidgetUtils.getUserWidgets();
// Get the current widgets and clone them before we modify them, otherwise
// we'll modify the content of the old event.
const userWidgets = JSON.parse(JSON.stringify(WidgetUtils.getUserWidgets()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object.assign({}, thingToClone) should also work and skips the overhead of JSON in the middle.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, although only shallow, although that would probably suffice in this case...

@dbkr dbkr merged commit 359d2fe into develop Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants