diff --git a/frontend/src/plugins/impl/anywidget/__tests__/host.test.ts b/frontend/src/plugins/impl/anywidget/__tests__/host.test.ts index b1759e3b6ec..35008d90ee2 100644 --- a/frontend/src/plugins/impl/anywidget/__tests__/host.test.ts +++ b/frontend/src/plugins/impl/anywidget/__tests__/host.test.ts @@ -277,25 +277,22 @@ describe("WidgetBinding receives a host in render props", () => { }); }); -describe("host-mounted views hydrate like display-mounted ones", () => { - it("replays current state to a child's render listeners", async () => { - const childId = asModelId("host-hydration-child"); +describe("host-mounted views", () => { + it("render with the child's current model state", async () => { + const childId = asModelId("host-child-state"); const parentController = new AbortController(); const getModuleSpy = vi.spyOn(WIDGET_DEF_REGISTRY, "getModule"); try { const childWidget = { render: vi.fn(({ model, el }) => { - el.textContent = "count is 5"; - model.on("change:count", () => { - el.textContent = `count is ${model.get("count")}`; - }); + el.textContent = `count is ${model.get("count")}`; }), }; const childModel = new Model({ count: 8 }, createMockComm()); WIDGET_REGISTRY.setModel(childId, childModel); WIDGET_REGISTRY.setSpec(childId, { - url: "./@file/10-host-hydration-child.js", - hash: "hash-host-hydration-child", + url: "./@file/10-host-child-state.js", + hash: "hash-host-child-state", }); getModuleSpy.mockResolvedValue({ default: childWidget }); diff --git a/frontend/src/plugins/impl/anywidget/__tests__/widget-binding.test.ts b/frontend/src/plugins/impl/anywidget/__tests__/widget-binding.test.ts index a197c7d9ea7..7b4f59ff095 100644 --- a/frontend/src/plugins/impl/anywidget/__tests__/widget-binding.test.ts +++ b/frontend/src/plugins/impl/anywidget/__tests__/widget-binding.test.ts @@ -337,13 +337,12 @@ describe("WidgetBinding", () => { { el: document.createElement("div") }, { signal: controller.signal }, ); - // Called once by the hydration replay at mount, once by the set. model.set("count", 1); - expect(onCount).toHaveBeenCalledTimes(2); + expect(onCount).toHaveBeenCalledTimes(1); controller.abort(); model.set("count", 2); - expect(onCount).toHaveBeenCalledTimes(2); + expect(onCount).toHaveBeenCalledTimes(1); }); it("should auto-clear initialize listeners on destroy", async () => { @@ -366,92 +365,54 @@ describe("WidgetBinding", () => { }); }); -describe("WidgetBinding hydration replay", () => { - it("replays current state to render listeners exactly once", async () => { +describe("WidgetBinding render listeners", () => { + it("fire only for changes after mount, never for pre-existing state", async () => { const model = new Model({ count: 8 }, createMockComm()); - const el = document.createElement("div"); - const widgetDef = { - render: vi.fn(({ model, el }) => { - // A widget view that starts with a local default and relies on - // change events for hydration. - el.textContent = "count is 5"; - model.on("change:count", () => { - el.textContent = `count is ${model.get("count")}`; - }); - }), - }; - const binding = await createBinding(widgetDef, model); - - const controller = new AbortController(); - await binding.createView({ el }, { signal: controller.signal }); - expect(el.textContent).toBe("count is 8"); - }); - - it("does not re-fire listeners of already-mounted views", async () => { - // Mounting view B must not double-paint view A: the replay is - // scoped to the listeners the new render attached. - const model = new Model({ count: 0 }, createMockComm()); - const listeners: Array> = []; + const onCount = vi.fn(); + const onAnyChange = vi.fn(); const widgetDef = { render: vi.fn(({ model }) => { - const listener = vi.fn(); - listeners.push(listener); - model.on("change:count", listener); + model.on("change:count", onCount); + model.on("change", onAnyChange); }), }; const binding = await createBinding(widgetDef, model); const controller = new AbortController(); - await binding.createView( { el: document.createElement("div") }, { signal: controller.signal }, ); - expect(listeners[0]).toHaveBeenCalledTimes(1); + expect(onCount).not.toHaveBeenCalled(); + expect(onAnyChange).not.toHaveBeenCalled(); - await binding.createView( - { el: document.createElement("div") }, - { signal: controller.signal }, - ); - // View B's listener hydrated once; view A's was left alone. - expect(listeners[1]).toHaveBeenCalledTimes(1); - expect(listeners[0]).toHaveBeenCalledTimes(1); + model.set("count", 9); + expect(onCount).toHaveBeenCalledTimes(1); + expect(onCount).toHaveBeenCalledWith(9); }); - it("replays the any-change event to its listeners", async () => { - const model = new Model({ count: 1 }, createMockComm()); - const onAnyChange = vi.fn(); + it("mounting a second view does not fire the first view's listeners", async () => { + const model = new Model({ count: 0 }, createMockComm()); + const listeners: Array> = []; const widgetDef = { render: vi.fn(({ model }) => { - model.on("change", onAnyChange); + const listener = vi.fn(); + listeners.push(listener); + model.on("change:count", listener); }), }; const binding = await createBinding(widgetDef, model); const controller = new AbortController(); + await binding.createView( { el: document.createElement("div") }, { signal: controller.signal }, ); - expect(onAnyChange).toHaveBeenCalledTimes(1); - }); - - it("does not replay initialize listeners", async () => { - // The guarantee is per-view: initialize listeners existed before - // any view, and replaying at them would fire once per mount. - const model = new Model({ count: 1 }, createMockComm()); - const initListener = vi.fn(); - const widgetDef = { - initialize: vi.fn(({ model }) => { - model.on("change:count", initListener); - }), - render: vi.fn(), - }; - const binding = await createBinding(widgetDef, model); - const controller = new AbortController(); await binding.createView( { el: document.createElement("div") }, { signal: controller.signal }, ); - expect(initListener).not.toHaveBeenCalled(); + expect(listeners[0]).not.toHaveBeenCalled(); + expect(listeners[1]).not.toHaveBeenCalled(); }); it("clears the element before rendering into it", async () => { diff --git a/frontend/src/plugins/impl/anywidget/model-proxy.ts b/frontend/src/plugins/impl/anywidget/model-proxy.ts index e80999f8a4e..ea2ca2cbdab 100644 --- a/frontend/src/plugins/impl/anywidget/model-proxy.ts +++ b/frontend/src/plugins/impl/anywidget/model-proxy.ts @@ -5,15 +5,6 @@ import type { ModelState } from "./types"; type ModelEventCallback = Parameters["on"]>[1]; -/** - * A listener registered through a model proxy (see the hydration - * replay in `WidgetBinding.createView`). - */ -export interface ProxyRegistration { - event: string; - callback: ModelEventCallback; -} - /** * Wrap a model so every `on()` call from inside `initialize` or `render` * is auto-tied to a lifetime `AbortSignal`. When the signal aborts, @@ -27,14 +18,11 @@ export interface ProxyRegistration { * The proxy is purely a host-side ergonomic helper. The widget author * still writes `model.on("change:foo", handler)` exactly as before; the * cleanup signal is supplied transparently. - * - * `onRegister`, when given, observes each `on()` registration. */ // oxlint-disable-next-line marimo/prefer-object-params -- concise internal helper used at protocol call sites export function modelProxy( model: AnyModel, signal: AbortSignal, - onRegister?: (registration: ProxyRegistration) => void, ): AnyModel { return { get(key) { @@ -55,7 +43,6 @@ export function modelProxy( signal.addEventListener("abort", () => model.off(name, callback), { once: true, }); - onRegister?.({ event: name, callback }); }, off(name?: string | null, callback?: ModelEventCallback | null): void { model.off(name ?? null, callback ?? null); diff --git a/frontend/src/plugins/impl/anywidget/widget-binding.ts b/frontend/src/plugins/impl/anywidget/widget-binding.ts index c71cb0c2d75..981739501d8 100644 --- a/frontend/src/plugins/impl/anywidget/widget-binding.ts +++ b/frontend/src/plugins/impl/anywidget/widget-binding.ts @@ -12,7 +12,7 @@ import { isTrustedVirtualFileUrl } from "@/plugins/core/trusted-url"; import { Logger } from "@/utils/Logger"; import type { Host } from "./host"; import type { Model } from "./model"; -import { modelProxy, type ProxyRegistration } from "./model-proxy"; +import { modelProxy } from "./model-proxy"; import type { ModelState } from "./types"; export const experimental: Experimental = { @@ -266,9 +266,8 @@ export class WidgetBinding { * when the caller's `signal` fires or the binding is destroyed, and * listeners registered inside `render` auto-clear on abort. * - * Hydration guarantee: listeners attached during `render` observe - * current model state exactly once, after `render` settles. Scoped - * to this view's listeners; re-firing at other views double-paints. + * `render` reads current state via `model.get`; change listeners + * observe only subsequent changes, matching Jupyter semantics. */ async createView( target: { el: HTMLElement }, @@ -305,11 +304,8 @@ export class WidgetBinding { // Each view gets a host scoped to its own signal so child views tear // down with this view. const host = this.#createHost(renderSignal); - const registrations: ProxyRegistration[] = []; const renderCleanup = await widget.render({ - model: modelProxy(this.#model, renderSignal, (registration) => - registrations.push(registration), - ), + model: modelProxy(this.#model, renderSignal), el: target.el, experimental, signal: renderSignal, @@ -333,7 +329,6 @@ export class WidgetBinding { once: true, }); } - this.#replayState(registrations, renderSignal); } #trackCleanup(cleanup: Cleanup, reason: string): Promise { @@ -343,31 +338,6 @@ export class WidgetBinding { return task; } - /** - * The hydration guarantee documented on `createView`. - */ - #replayState( - registrations: readonly ProxyRegistration[], - renderSignal: AbortSignal, - ): void { - const changePrefix = "change:"; - for (const { event, callback } of registrations) { - if (renderSignal.aborted) { - return; - } - try { - if (event.startsWith(changePrefix)) { - const key = event.slice(changePrefix.length); - callback(this.#model.get(key)); - } else if (event === "change") { - callback(); - } - } catch (error) { - Logger.error("[WidgetBinding] Error replaying state", error); - } - } - } - /** * Destroy this generation, running initialize/render cleanups and * clearing listeners registered through its model proxies.