From 61273068ed6081a25110bd4175bbcaea6c3e7063 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Mon, 28 Jun 2021 12:48:18 -0700 Subject: [PATCH] Store the values that have been synced to the kernel, but are still listed as changed, so they do not get buffered again to be synced to the kernel. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 https://github.com/jupyter-widgets/ipywidgets/pull/3216#issuecomment-868396472 and the following comments for more notes on this issue. --- packages/base/src/widget.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/base/src/widget.ts b/packages/base/src/widget.ts index 83a90d4c77..a4b7ac2f2f 100644 --- a/packages/base/src/widget.ts +++ b/packages/base/src/widget.ts @@ -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; } @@ -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 = {}; } } @@ -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;