cleanup initialize and render

packages/anywidget/src/model.js

@@ -8,11 +8,9 @@ import * as utils from "./util.js";
 /** @template {Record<string, unknown>} T */
 export class Model {
 	/** @type {import("./types.js").Comm=} */
-	#comm;
+	comm;
 	/** @type {Omit<import("./types.js").ModelOptions, "comm">} */
 	#options;
-	/** @type {Map<any, { [evt_name: string]: Map<() => void, (event: Event) => void> }>} */
-	#listeners = new Map();
 	/** @type {T} */
 	#state;
 	/** @type {Set<string>} */
@@ -21,13 +19,17 @@ export class Model {
 	#field_serializers;
 	/** @type {EventTarget} */
 	#events = new EventTarget();
+	/** @type {Map<any, { [evt_name: string]: Map<() => void, (event: Event) => void> }>} */
+	#listeners = new Map();
 
 	// NOTE: (from Jupyter Team): keep track of the msg id for each attr for updates
 	// we send out so that we can ignore old messages that we send in
 	// order to avoid 'drunken' sliders going back and forward
 	/** @type {Map<string, string>} */
 	#expected_echo_msg_ids = new Map();
 
+	#closed = false;
+
 	// NOTE: Required for the WidgetManager to know when the model is ready
 	/** @type {Promise<void>} */
 	state_change;
@@ -48,15 +50,18 @@ export class Model {
 				serialize(layout) {
 					return JSON.parse(JSON.stringify(layout));
 				},
-				/** @param {string} layout */
+				/**
+				 * @param {string} layout
+				 * @param {import("./types.js").WidgetManager} widget_manager
+				 */
 				deserialize(layout, widget_manager) {
 					return widget_manager.get_model(layout.slice("IPY_MODEL_".length));
 				},
 			},
 		};
-		this.#comm = options.comm;
-		this.#comm?.on_msg(this.#handle_comm_msg.bind(this));
-		this.#comm?.on_close(this.#handle_comm_close.bind(this));
+		this.comm = options.comm;
+		this.comm?.on_msg(this.#handle_comm_msg.bind(this));
+		this.comm?.on_close(this.#handle_comm_close.bind(this));
 		this.state_change = this.#deserialize(state).then((de) => {
 			this.#state = de;
 		});
@@ -66,8 +71,17 @@ export class Model {
 		return this.#options.widget_manager;
 	}
 
+	/** @param {boolean} update */
+	set comm_live(update) {
+		// NOTE: JupyterLab seems to try to set this. The only sensible behavior I can think of
+		// is to set the comm to undefined if the update is false, and do nothing otherwise.
+		if (update === false) {
+			this.comm = undefined;
+		}
+	}
+
 	get comm_live() {
-		return !!this.#comm;
+		return !!this.comm;
 	}
 
 	get #msg_buffer() {
@@ -136,6 +150,9 @@ export class Model {
 	 * @param {import("./types.js").CommMessage} msg - the comm message.
 	 */
 	async #handle_comm_msg(msg) {
+		if (!this.comm) {
+			return;
+		}
 		if (utils.is_update_msg(msg)) {
 			await this.#handle_update(msg);
 			return;
@@ -201,17 +218,17 @@ export class Model {
 	 * @returns - a promise that is fulfilled when all the associated views have been removed.
 	 */
 	async #handle_comm_close() {
-		// can only be closed once.
-		if (!this.#comm) return;
-		this.#comm.close();
-		this.#comm = undefined;
-		this.#emit("comm:close");
-		this.off();
-		this.#listeners.clear();
+		this.trigger("comm:close");
+		if (this.#closed) {
+			return;
+		}
+		this.#closed = true;
+		this.comm?.close();
+		this.comm = undefined;
 		for await (let view of Object.values(this.views)) {
 			view.remove();
 		}
-		this.views = {};
+		this.trigger("destroy");
 	}
 
 	/**
@@ -236,7 +253,7 @@ export class Model {
 	}
 
 	async save_changes() {
-		if (!this.#comm) return;
+		if (!this.comm) return;
 		/** @type {Partial<T>} */
 		let to_send = {};
 		for (let key of this.#need_sync) {
@@ -246,7 +263,7 @@ export class Model {
 		let serialized = await this.serialize(to_send);
 		this.#need_sync.clear();
 		let { state, buffer_paths, buffers } = utils.extract_buffers(serialized);
-		this.#comm.send(
+		this.comm.send(
 			{ method: "update", state, buffer_paths },
 			undefined,
 			{},
@@ -311,7 +328,6 @@ export class Model {
 	 * @param {unknown} [scope]
 	 */
 	off(event, callback, scope) {
-		let callbacks = [];
 		for (let [s, scope_listeners] of this.#listeners.entries()) {
 			if (scope && scope !== s) {
 				continue;
@@ -324,20 +340,11 @@ export class Model {
 					if (callback && callback !== cb) {
 						continue;
 					}
-					callbacks.push({ event: e, handler });
+					this.#events.removeEventListener(e, handler);
 					listeners.delete(cb);
 				}
-				if (!listeners.size) {
-					delete scope_listeners[e];
-				}
-			}
-			if (Object.keys(scope_listeners).length == 0) {
-				this.#listeners.delete(s);
 			}
 		}
-		for (let { event, handler } of callbacks) {
-			this.#events.removeEventListener(event, handler);
-		}
 	}
 
 	/**
@@ -347,13 +354,16 @@ export class Model {
 	 * @param {ArrayBuffer[]} [buffers] - An array of ArrayBuffers to send as part of the message.
 	 */
 	send(content, callbacks, buffers) {
-		if (!this.#comm) return;
-		this.#comm.send({ method: "custom", content }, callbacks, {}, buffers);
+		if (!this.comm) return;
+		this.comm.send({ method: "custom", content }, callbacks, {}, buffers);
 	}
 
 	/** @param {string} event */
 	trigger(event) {
-		utils.assert(event === "destroy", "Only 'destroy' event is supported");
+		utils.assert(
+			event === "destroy" || event === "comm:close",
+			"[anywidget] Only 'destroy' or 'comm:close' event is supported `Model.trigger`",
+		);
 		this.#emit(event);
 	}
 

packages/anywidget/src/runtime.js

@@ -75,7 +75,7 @@ export class Runtime {
 					cleanup?.();
 					// Clear all previous event listeners from this hook.
 					model.off(null, null, view);
-					util.empty_element(view.el);
+					util.empty(view.el);
 					if (widget_result.state === "error") {
 						util.throw_anywidget_error(widget_result.error);
 					}

packages/anywidget/src/util.js

@@ -347,7 +347,7 @@ export function throw_anywidget_error(source) {
 }
 
 /** @param {HTMLElement} el */
-export function empty_element(el) {
+export function empty(el) {
 	while (el.firstChild) {
 		el.removeChild(el.firstChild);
 	}

packages/anywidget/src/view.js

@@ -1,18 +1,18 @@
-import * as utils from "./util.js";
+import * as util from "./util.js";
 import { Widget } from "@lumino/widgets";
 
-/**
- * @template {Record<string, unknown>} T
- * @typedef {import("./model.js").Model<T>} Model
- */
-
 /**
  * @typedef LuminoMessage
  * @property {string} type
  * @property {boolean} isConflatable
  * @property {(msg: LuminoMessage) => boolean} conflate
  */
 
+/**
+ * @template {Record<string, unknown>} T
+ * @typedef {import("./model.js").Model<T>} Model
+ */
+
 /**
  * @template {Record<string, unknown>} T
  * @typedef ViewOptions
@@ -31,14 +31,14 @@ export class View {
 	model;
 	/** @type {Record<string, unknown>} */
 	options;
-	/** @type {() => void} */
 	#remove_callback = () => {};
 
 	/** @param {ViewOptions<T>} options */
 	constructor({ el, model, ...options }) {
 		this.el = el ?? document.createElement("div");
 		this.model = model;
 		this.options = options;
+		// TODO: We should try to drop the Lumino dependency. However, this seems required for all widgets.
 		this.luminoWidget = new Widget({ node: this.el });
 	}
 
@@ -48,32 +48,37 @@ export class View {
 	 * @param {() => void} callback
 	 */
 	listenTo(model, name, callback) {
-		utils.assert(
+		util.assert(
 			name === "destroy",
-			"[anywidget]: Only 'destroy' event is supported in `listenTo`.",
+			"[anywidget] Only 'destroy' event is supported in `View.listenTo`",
 		);
-		model.once("destroy", callback);
 	}
 
 	/**
 	 * @param {"remove"} name
 	 * @param {() => void} callback
 	 */
 	once(name, callback) {
-		utils.assert(
+		util.assert(
 			name === "remove",
-			"[anywidget]: Only 'remove' event is supported in `once`.",
+			"[anywidget] Only 'remove' event is supported in `View.once`",
 		);
 		this.#remove_callback = callback;
 	}
 
 	remove() {
 		this.#remove_callback();
+		util.empty(this.el);
 		this.el.remove();
+		this.model.off(null, null, this);
 	}
 
 	/**
 	 * Render the view.
+	 *
+	 * Should be overridden by subclasses.
+	 *
+	 * @returns {Promise<void>}
 	 */
 	async render() {}
 }

packages/anywidget/src/widget.js

@@ -16,6 +16,7 @@ export default function () {
 		constructor(...args) {
 			super(...args);
 			let runtime = new Runtime(this);
+			window.model = this;
 			this.once("destroy", () => {
 				try {
 					runtime.dispose();
@@ -29,16 +30,15 @@ export default function () {
 
 	/** @extends {View<any>} */
 	class AnyView extends View {
-		/** @type {undefined | (() => void)} */
-		#dispose = undefined;
+		/** @type {() => void} */
+		#dispose = () => {};
 		async render() {
 			let runtime = RUNTIMES.get(/** @type {any} */ (this.model));
 			util.assert(runtime, "[anywidget] runtime not found.");
-			util.assert(!this.#dispose, "[anywidget] dispose already set.");
 			this.#dispose = await runtime.create_view(this);
 		}
 		remove() {
-			this.#dispose?.();
+			this.#dispose();
 			super.remove();
 		}
 	}