Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
cpsievert committed Nov 18, 2024
1 parent eedb1b8 commit 63d44d9
Showing 3 changed files with 27 additions and 28 deletions.
40 changes: 16 additions & 24 deletions js/src/output.ts
Original file line number Diff line number Diff line change
@@ -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
@@ -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;

@@ -213,14 +195,24 @@ Shiny.addCustomMessageHandler("shinywidgets_comm_msg", (msg_txt) => {
});


// When `widget.close()` happens server-side, don't `.close()` client-side until the
// 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 = [];
// Handle the closing of a widget/comm/model
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
13 changes: 10 additions & 3 deletions shinywidgets/_comm.py
Original file line number Diff line number Diff line change
@@ -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:
2 changes: 1 addition & 1 deletion shinywidgets/static/output.js

0 comments on commit 63d44d9

Please sign in to comment.