Skip to content

Commit

Permalink
Send close messages on_flushed() instead of on_flush() to avoid closi…
Browse files Browse the repository at this point in the history
…ng before render
  • Loading branch information
cpsievert committed Nov 18, 2024
1 parent eedb1b8 commit a6886fb
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 24 deletions.
35 changes: 15 additions & 20 deletions js/src/output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import type { ErrorsMessageValue } from 'rstudio-shiny/srcts/types/src/shiny/shi
******************************************************************************/

class OutputManager extends HTMLManager {
orphaned_models: string[];
// In a soon-to-be-released version of @jupyter-widgets/html-manager,
// display_view()'s first "dummy" argument will be removed... this shim simply
// makes it so that our manager can work with either version
Expand Down Expand Up @@ -101,23 +100,6 @@ class IPyWidgetOutput extends Shiny.OutputBinding {
const view = await manager.create_view(model, {});
await manager.display_view(view, {el: el});

// If the model was orphaned, close it (and thus, the view as well) now
if (manager.orphaned_models.length > 0) {
for (const model_id of manager.orphaned_models) {
const model = await manager.get_model(model_id);
if (model) {
// Closing the model removes the previous view
await model.close();
// .close() isn't enough to remove manager's reference to it,
// and apparently the only way to remove it is through the `comm:close` event
// https://github.com/jupyter-widgets/ipywidgets/blob/303cae4/packages/base-manager/src/manager-base.ts#L330-L337
// https://github.com/jupyter-widgets/ipywidgets/blob/303cae4/packages/base/src/widget.ts#L251-L253
model.trigger("comm:close");
}
}
manager.orphaned_models = [];
}

// The ipywidgets container (.lmWidget)
const lmWidget = el.children[0] as HTMLElement;

Expand Down Expand Up @@ -217,10 +199,23 @@ Shiny.addCustomMessageHandler("shinywidgets_comm_msg", (msg_txt) => {
// next render. This is because we currently trigger a close _during_ the server-side
// render, and thus, immediately closing the model removes the view before a new one is
// ready.
manager.orphaned_models = [];
Shiny.addCustomMessageHandler("shinywidgets_comm_close", (msg_txt) => {
const msg = jsonParse(msg_txt);
manager.orphaned_models.push(msg.content.comm_id);
const id = msg.content.comm_id;
const model = manager.get_model(id);
if (!model) {
console.error(`Couldn't close model ${id} because it doesn't exist.`);
return;
}
model.then(m => {
// Closing the model removes the previous view
m.close();
// .close() isn't enough to remove manager's reference to it,
// and apparently the only way to remove it is through the `comm:close` event
// https://github.com/jupyter-widgets/ipywidgets/blob/303cae4/packages/base-manager/src/manager-base.ts#L330-L337
// https://github.com/jupyter-widgets/ipywidgets/blob/303cae4/packages/base/src/widget.ts#L251-L253
m.trigger("comm:close");
});
});

// At least currently, all widgets must be created within a session scope, so we can
Expand Down
13 changes: 10 additions & 3 deletions shinywidgets/_comm.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,17 @@ def _publish_msg(
def _send():
run_coro_hybrid(session.send_custom_message(msg_type, msg_txt)) # type: ignore

# N.B., if we don't do this on flush, then if you initialize a widget
# outside of a reactive context, run_coro_sync() will complain with
# N.B., if messages are sent immediately, run_coro_sync() could fail with
# 'async function yielded control; it did not finish in one iteration.'
session.on_flush(_send)
# if executed outside of a reactive context.
if msg_type == "shinywidgets_comm_close":
# The primary way widgets are closed are when a new widget is rendered in
# its place (see render_widget_base). By sending close on_flushed(), we
# ensure to close the 'old' widget after the new one is created. (avoiding a
# "flicker" of the old widget being removed before the new one is created)
session.on_flushed(_send)
else:
session.on_flush(_send)

# This is the method that ipywidgets.widgets.Widget uses to respond to client-side changes
def on_msg(self, callback: MsgCallback) -> None:
Expand Down
Loading

0 comments on commit a6886fb

Please sign in to comment.