Skip to content

Commit 71fca1b

Browse files
cpsievertclaude
andcommitted
fix: resolve "no comm channel defined" error with plotly 6.x
Fixes posit-dev/py-shiny#2156 Plotly 6.0 switched FigureWidget to use anywidget, which doesn't have a destroy() method. The previous cleanup logic only called v.remove() for views with destroy(), causing anywidget-based views to skip removal before m.close(). Since ipywidgets' close() deletes the comm before removing views, the view removal would fail when trying to sync. The fix ensures all views are removed while the comm is still available. Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 23cbe34 commit 71fca1b

File tree

2 files changed

+13
-10
lines changed

2 files changed

+13
-10
lines changed

js/src/output.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,11 @@ Shiny.addCustomMessageHandler("shinywidgets_comm_close", async (msg_txt) => {
222222
try {
223223
const m = await model;
224224

225-
// Before .close()ing the model (which will .remove() each view), do some
226-
// additional cleanup that .remove() might miss
225+
// Remove all views BEFORE calling m.close(). This is critical because:
226+
// 1. m.close() deletes the comm before removing views
227+
// 2. When views are removed, they fire a 'remove' event that tries to sync _view_count
228+
// 3. If the comm is already deleted, the sync fails with "no comm channel defined"
229+
// By removing views first (while comm is still available), we avoid this error.
227230
await Promise.all(
228231
Object.values(m.views).map(async (viewPromise) => {
229232
try {
@@ -233,15 +236,15 @@ Shiny.addCustomMessageHandler("shinywidgets_comm_close", async (msg_txt) => {
233236
// https://github.com/plotly/plotly.py/pull/3805/files#diff-259c92d
234237
if (hasMethod<DestroyMethod>(v, 'destroy')) {
235238
v.destroy();
236-
// Also, empirically, when this destroy() is relevant, it also helps to
237-
// delete the view's reference to the model, I think this is the only
238-
// way to drop the resize event listener (see the diff in the link above)
239-
// https://github.com/posit-dev/py-shinywidgets/issues/166
240-
delete v.model;
241-
// Ensure sure the lm-Widget container is also removed
242-
v.remove();
243239
}
244240

241+
// Remove the view while the comm is still available (so _view_count can sync)
242+
v.remove();
243+
244+
// After remove(), delete the view's reference to the model to help with
245+
// memory cleanup and to drop the resize event listener
246+
// https://github.com/posit-dev/py-shinywidgets/issues/166
247+
delete v.model;
245248

246249
} catch (err) {
247250
console.error("Error cleaning up view:", err);

0 commit comments

Comments
 (0)