From d316274c3490666b37cfaec43993caeeb70c3b9f Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 10 Apr 2026 11:51:22 -0500 Subject: [PATCH 1/3] wip: investigate issue 215 heavy ipyleaflet rerender --- js/src/output.ts | 159 ++++++++++-------- shinywidgets/_shinywidgets.py | 153 +++++++++++++---- shinywidgets/static/output.js | 2 +- .../ipyleaflet_rerender_cleanup/app.py | 11 +- .../test_ipyleaflet_rerender_cleanup.py | 47 +++++- 5 files changed, 265 insertions(+), 107 deletions(-) diff --git a/js/src/output.ts b/js/src/output.ts index 8cf3cf1..ed808d6 100644 --- a/js/src/output.ts +++ b/js/src/output.ts @@ -63,6 +63,16 @@ const shinyRequireLoader = async function(moduleName: string, moduleVersion: str } const manager = new OutputManager({ loader: shinyRequireLoader }); +let managerTaskQueue: Promise = Promise.resolve(); + +function enqueueManagerTask(task: () => Promise | T): Promise { + const queuedTask = managerTaskQueue.then(() => task()); + managerTaskQueue = queuedTask.then( + () => undefined, + () => undefined, + ); + return queuedTask; +} /****************************************************************************** @@ -95,6 +105,7 @@ class IPyWidgetOutput extends Shiny.OutputBinding { // At this time point, we should've already handled an 'open' message, and so // the model should be ready to use + await managerTaskQueue; const model = await manager.get_model(data.model_id); if (!model) { throw new Error(`No model found for id ${data.model_id}`); @@ -182,95 +193,103 @@ if (taskQueue) { // Initialize the comm and model when a new widget is created // This is basically our version of https://github.com/jupyterlab/jupyterlab/blob/d33de15/packages/services/src/kernel/default.ts#L1144-L1176 Shiny.addCustomMessageHandler("shinywidgets_comm_open", (msg_txt) => { - setBaseURL(); - const msg = jsonParse(msg_txt); - Shiny.renderDependencies(msg.content.html_deps); - const comm = new ShinyComm(msg.content.comm_id); - manager.handle_comm_open(comm, msg); + void enqueueManagerTask(async () => { + setBaseURL(); + const msg = jsonParse(msg_txt); + Shiny.renderDependencies(msg.content.html_deps); + const comm = new ShinyComm(msg.content.comm_id); + await manager.handle_comm_open(comm, msg); + }).catch((err) => { + console.error("Error opening widget model:", err); + }); }); // Handle any mutation of the model (e.g., add a marker to a map, without a full redraw) // Basically out version of https://github.com/jupyterlab/jupyterlab/blob/d33de15/packages/services/src/kernel/default.ts#L1200-L1215 Shiny.addCustomMessageHandler("shinywidgets_comm_msg", async (msg_txt) => { - const msg = jsonParse(msg_txt); - const id = msg.content.comm_id; - const model = manager.get_model(id); - if (!model) { - console.error(`Couldn't handle message for model ${id} because it doesn't exist.`); - return; - } - try { - const m = await model; - // @ts-ignore for some reason IClassicComm doesn't have this method, but we do - m.comm.handle_msg(msg); - } catch (err) { - console.error("Error handling message:", err); - } + void enqueueManagerTask(async () => { + const msg = jsonParse(msg_txt); + const id = msg.content.comm_id; + const model = manager.get_model(id); + if (!model) { + console.error(`Couldn't handle message for model ${id} because it doesn't exist.`); + return; + } + try { + const m = await model; + // @ts-ignore for some reason IClassicComm doesn't have this method, but we do + m.comm.handle_msg(msg); + } catch (err) { + console.error("Error handling message:", err); + } + }); }); // Handle the closing of a widget/comm/model Shiny.addCustomMessageHandler("shinywidgets_comm_close", async (msg_txt) => { - const msg = jsonParse(msg_txt); - 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; - } + void enqueueManagerTask(async () => { + const msg = jsonParse(msg_txt); + 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; + } - try { - const m = await model; - - // Some widget views need explicit teardown before model.close() removes them. - if (m.views) { - await Promise.all( - Object.values(m.views).map(async (viewPromise) => { - try { - const v = await viewPromise; - - // Plotly-backed views can leave DOM state and listeners behind unless - // destroy() runs before the view is removed. - if (hasMethod(v, 'destroy')) { - v.destroy(); - // Clearing the back-reference prevents later teardown from touching a - // model that is already being closed. - delete v.model; - v.remove(); + try { + const m = await model; + + // Some widget views need explicit teardown before model.close() removes them. + if (m.views) { + await Promise.all( + Object.values(m.views).map(async (viewPromise) => { + try { + const v = await viewPromise; + + // Plotly-backed views can leave DOM state and listeners behind unless + // destroy() runs before the view is removed. + if (hasMethod(v, 'destroy')) { + v.destroy(); + // Clearing the back-reference prevents later teardown from touching a + // model that is already being closed. + delete v.model; + v.remove(); + } + + + } catch (err) { + console.error("Error cleaning up view:", err); } + }) + ); + } + // View removal updates _view_count. Mark the comm as inactive first so that + // save_changes() does not try to send those updates after the comm is gone. + m.comm_live = false; - } catch (err) { - console.error("Error cleaning up view:", err); + // Close model after all views are cleaned up. + try { + await m.close(); + } catch (closeErr) { + if (!isIgnorableTeardownError(closeErr)) { + console.error("Unexpected error while closing model:", closeErr); } - }) - ); - } - - // View removal updates _view_count. Mark the comm as inactive first so that - // save_changes() does not try to send those updates after the comm is gone. - m.comm_live = false; - - // Close model after all views are cleaned up. - try { - await m.close(); - } catch (closeErr) { - if (!isIgnorableTeardownError(closeErr)) { - console.error("Unexpected error while closing model:", closeErr); } - } - // HTMLManager releases the model from its registry on comm:close. - try { - m.trigger("comm:close"); - } catch (triggerErr) { - if (!isIgnorableTeardownError(triggerErr)) { - console.error("Unexpected error while triggering comm:close:", triggerErr); + // HTMLManager releases the model from its registry on comm:close. + try { + m.trigger("comm:close"); + } catch (triggerErr) { + if (!isIgnorableTeardownError(triggerErr)) { + console.error("Unexpected error while triggering comm:close:", triggerErr); + } } + } catch (err) { + console.error("Error during model cleanup:", err); } - } catch (err) { - console.error("Error during model cleanup:", err); - } + }); }); $(document).on("shiny:disconnected", () => { diff --git a/shinywidgets/_shinywidgets.py b/shinywidgets/_shinywidgets.py index e196fd1..afead5d 100644 --- a/shinywidgets/_shinywidgets.py +++ b/shinywidgets/_shinywidgets.py @@ -4,6 +4,7 @@ import json import os from contextlib import contextmanager +from dataclasses import dataclass from typing import TYPE_CHECKING, Any, Optional, Sequence, Union, cast from uuid import uuid4 from weakref import WeakSet @@ -88,6 +89,8 @@ def _(): def _cleanup_session_state(): SESSIONS.remove(session) + PENDING_WIDGET_OPENS.pop(session.id, None) + OPEN_QUEUE_SCHEDULED.discard(session.id) # Cleanup any widgets that were created in this session for id in SESSION_WIDGET_ID_MAP[session.id]: widget = WIDGET_INSTANCE_MAP.get(id) @@ -117,35 +120,24 @@ def _cleanup_session_state(): # comm, we just set the comm to a dummy comm for now (to avoid unnecessary work) w.comm = OrphanedShinyComm(id) - # Schedule the opening of the comm to happen sometime after this init function. - # This is important for widgets like plotly that do additional initialization that - # is required to get a valid widget state. - @reactive.effect(priority=99999) - def _open_shiny_comm(): - - # Call _repr_mimebundle_() before get_state() since it may modify the widget - # in an important way (unfortunately, it does for plotly) - # # https://github.com/plotly/plotly.py/blob/0089f32/packages/python/plotly/plotly/basewidget.py#L734-L738 - if hasattr(w, "_repr_mimebundle_") and callable(w._repr_mimebundle_): - w._repr_mimebundle_() - - # Now, get the state - state, buffer_paths, buffers = _remove_buffers(w.get_state()) - - # Initialize the comm -- this sends widget state to the frontend - with widget_comm_patch(): - w.comm = ShinyComm( - comm_id=id, - comm_manager=COMM_MANAGER, - target_name="jupyter.widgets", - data={"state": state, "buffer_paths": buffer_paths}, - buffers=cast(BufferType, buffers), - # TODO: should this be hard-coded? - metadata={"version": __protocol_version__}, - html_deps=session._process_ui(TagList(widget_dep))["deps"], - ) - - _open_shiny_comm.destroy() + PENDING_WIDGET_OPENS.setdefault(session.id, []).append( + PendingWidgetOpen(widget=w, widget_dep=widget_dep) + ) + if session.id not in OPEN_QUEUE_SCHEDULED: + OPEN_QUEUE_SCHEDULED.add(session.id) + + @reactive.effect(priority=99999) + def _open_pending_widget_comms() -> None: + OPEN_QUEUE_SCHEDULED.discard(session.id) + pending = PENDING_WIDGET_OPENS.pop(session.id, []) + prepared = [ + _prepare_pending_widget_open(session, pending_open) + for pending_open in pending + ] + for pending_open in _order_pending_widget_opens(prepared): + _open_shiny_comm(pending_open) + + _open_pending_widget_comms.destroy() # If the widget initialized in a reactive _output_ context, then cleanup the widget # when the context gets invalidated. Use the render output's Context (captured by @@ -199,11 +191,114 @@ def on_close(): # Dictionary mapping session id to widget ids # The key is the session id, and the value is a list of widget ids SESSION_WIDGET_ID_MAP: dict[str, list[str]] = {} +PENDING_WIDGET_OPENS: dict[str, list["PendingWidgetOpen"]] = {} +OPEN_QUEUE_SCHEDULED: set[str] = set() # Dictionary of all "active" widgets (ipywidgets automatically adds to this dictionary as # new widgets are created, but they won't get removed until the widget is explictly closed) WIDGET_INSTANCE_MAP = cast(dict[str, Widget], Widget.widgets) + +@dataclass +class PendingWidgetOpen: + widget: Widget + widget_dep: Any + + +@dataclass +class PreparedWidgetOpen: + widget: Widget + comm_id: str + state: dict[str, object] + buffer_paths: list[object] + buffers: BufferType + html_deps: list[object] + child_comm_ids: set[str] + + +def _prepare_pending_widget_open( + session: Session, pending: PendingWidgetOpen +) -> PreparedWidgetOpen: + widget = pending.widget + comm_id = cast(str, widget._model_id) + + if hasattr(widget, "_repr_mimebundle_") and callable(widget._repr_mimebundle_): + widget._repr_mimebundle_() + + state, buffer_paths, buffers = _remove_buffers(widget.get_state()) + return PreparedWidgetOpen( + widget=widget, + comm_id=comm_id, + state=state, + buffer_paths=buffer_paths, + buffers=cast(BufferType, buffers), + html_deps=session._process_ui(TagList(pending.widget_dep))["deps"], + child_comm_ids=_find_widget_model_refs(state), + ) + + +def _open_shiny_comm(pending: PreparedWidgetOpen) -> None: + with widget_comm_patch(): + pending.widget.comm = ShinyComm( + comm_id=pending.comm_id, + comm_manager=COMM_MANAGER, + target_name="jupyter.widgets", + data={"state": pending.state, "buffer_paths": pending.buffer_paths}, + buffers=pending.buffers, + metadata={"version": __protocol_version__}, + html_deps=pending.html_deps, + ) + + +def _order_pending_widget_opens( + pending: Sequence[PreparedWidgetOpen], +) -> list[PreparedWidgetOpen]: + pending_by_id = {widget.comm_id: widget for widget in pending} + ordered: list[PreparedWidgetOpen] = [] + visited: set[str] = set() + visiting: set[str] = set() + + def visit(comm_id: str) -> None: + if comm_id in visited or comm_id not in pending_by_id: + return + if comm_id in visiting: + return + + visiting.add(comm_id) + widget = pending_by_id[comm_id] + for child_comm_id in widget.child_comm_ids: + visit(child_comm_id) + visiting.remove(comm_id) + visited.add(comm_id) + ordered.append(widget) + + for widget in pending: + visit(widget.comm_id) + + return ordered + + +def _find_widget_model_refs(value: object) -> set[str]: + refs: set[str] = set() + + def collect(x: object) -> None: + if isinstance(x, str): + if x.startswith("IPY_MODEL_"): + refs.add(x.removeprefix("IPY_MODEL_")) + return + + if isinstance(x, dict): + for item in x.values(): + collect(item) + return + + if isinstance(x, (list, tuple, set)): + for item in x: + collect(item) + + collect(value) + return refs + # -------------------------------------- # Reactivity # -------------------------------------- diff --git a/shinywidgets/static/output.js b/shinywidgets/static/output.js index 44211c0..b5365b9 100644 --- a/shinywidgets/static/output.js +++ b/shinywidgets/static/output.js @@ -36,7 +36,7 @@ eval("__webpack_require__.r(__webpack_exports__);\n/* harmony export */ __webpac \***********************/ /***/ ((__unused_webpack_module, __webpack_exports__, __webpack_require__) => { -eval("__webpack_require__.r(__webpack_exports__);\n/* harmony import */ var _jupyter_widgets_html_manager__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! @jupyter-widgets/html-manager */ \"@jupyter-widgets/html-manager\");\n/* harmony import */ var _jupyter_widgets_html_manager__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(_jupyter_widgets_html_manager__WEBPACK_IMPORTED_MODULE_0__);\n/* harmony import */ var _comm__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! ./comm */ \"./src/comm.ts\");\n/* harmony import */ var _utils__WEBPACK_IMPORTED_MODULE_2__ = __webpack_require__(/*! ./utils */ \"./src/utils.ts\");\nvar _a;\n\n\n\n/******************************************************************************\n * Define a custom HTMLManager for use with Shiny\n ******************************************************************************/\nclass OutputManager extends _jupyter_widgets_html_manager__WEBPACK_IMPORTED_MODULE_0__.HTMLManager {\n // In a soon-to-be-released version of @jupyter-widgets/html-manager,\n // display_view()'s first \"dummy\" argument will be removed... this shim simply\n // makes it so that our manager can work with either version\n // https://github.com/jupyter-widgets/ipywidgets/commit/159bbe4#diff-45c126b24c3c43d2cee5313364805c025e911c4721d45ff8a68356a215bfb6c8R42-R43\n async display_view(view, options) {\n const n_args = super.display_view.length;\n if (n_args === 3) {\n return super.display_view({}, view, options);\n }\n else {\n // @ts-ignore\n return super.display_view(view, options);\n }\n }\n}\n// Define our own custom module loader for Shiny\nconst shinyRequireLoader = async function (moduleName, moduleVersion) {\n // shiny provides a shim of require.js which allows