Skip to content

Commit

Permalink
Store the values that have been synced to the kernel, but are still l…
Browse files Browse the repository at this point in the history
…isted as changed, so they do not get buffered again to be synced to the kernel.

This fixes a problem that comes up when we have a recursive call to .set (for example, when a top-level .set call triggers an event which leads to another .set call), and the state is synced while in this recursive call. In short, the end of every .set call involves copying over the changed attributes to _buffered_state_diff. However, if we’ve already sent the state to the kernel during this recursive .set call (which clears _buffered_state_diff), before this PR we still copied over the state change values another time into _buffered_state_diff.

This PR instead keeps track of the last values we have sent to the kernel during a recursive .set call, so that we don’t redundantly copy these values into the buffer again.

See #3216 (comment) and the following comments for more notes on this issue.
  • Loading branch information
jasongrout committed Jun 28, 2021
1 parent b0251d2 commit 6127306
Showing 1 changed file with 21 additions and 0 deletions.
21 changes: 21 additions & 0 deletions packages/base/src/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,25 @@ export class WidgetModel extends Backbone.Model {
}
}

// _buffered_state_diff_synced lists things that have already been sent to the kernel during a top-level call to .set(), so we don't need to buffer these things either.
if (this._buffered_state_diff_synced) {
for (const key of Object.keys(this._buffered_state_diff_synced)) {
if (attrs[key] === this._buffered_state_diff_synced[key]) {
delete attrs[key];
}
}
}

this._buffered_state_diff = utils.assign(
this._buffered_state_diff,
attrs
);
}

// If this ended a top-level call to .set, then reset _buffered_state_diff_synced
if ((this as any)._changing === false) {
this._buffered_state_diff_synced = {};
}
return return_value;
}

Expand Down Expand Up @@ -516,6 +530,12 @@ export class WidgetModel extends Backbone.Model {
options.callbacks = callbacks;
}
this.save(this._buffered_state_diff, options);

// If we are currently in a .set() call, save what state we have synced
// to the kernel so we don't buffer it again as we come out of the .set call.
if ((this as any)._changing) {
utils.assign(this._buffered_state_diff_synced, this._buffered_state_diff);
}
this._buffered_state_diff = {};
}
}
Expand Down Expand Up @@ -595,6 +615,7 @@ export class WidgetModel extends Backbone.Model {
private _closed: boolean;
private _state_lock: any;
private _buffered_state_diff: any;
private _buffered_state_diff_synced: any;
private _msg_buffer: any;
private _msg_buffer_callbacks: any;
private _pending_msgs: number;
Expand Down

0 comments on commit 6127306

Please sign in to comment.