Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions frontend/src/plugins/impl/anywidget/__tests__/host.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ModelState>({ 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 });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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<ModelState>({ 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<ModelState>({ count: 0 }, createMockComm());
const listeners: Array<ReturnType<typeof vi.fn>> = [];
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<ModelState>({ 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<ModelState>({ count: 0 }, createMockComm());
const listeners: Array<ReturnType<typeof vi.fn>> = [];
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<ModelState>({ 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 () => {
Expand Down
13 changes: 0 additions & 13 deletions frontend/src/plugins/impl/anywidget/model-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,6 @@ import type { ModelState } from "./types";

type ModelEventCallback = Parameters<AnyModel<ModelState>["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,
Expand All @@ -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<T extends ModelState>(
model: AnyModel<T>,
signal: AbortSignal,
onRegister?: (registration: ProxyRegistration) => void,
): AnyModel<T> {
return {
get(key) {
Expand All @@ -55,7 +43,6 @@ export function modelProxy<T extends ModelState>(
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);
Expand Down
38 changes: 4 additions & 34 deletions frontend/src/plugins/impl/anywidget/widget-binding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -266,9 +266,8 @@ export class WidgetBinding<T extends ModelState = ModelState> {
* 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 },
Expand Down Expand Up @@ -305,11 +304,8 @@ export class WidgetBinding<T extends ModelState = ModelState> {
// 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),
Comment thread
manzt marked this conversation as resolved.
el: target.el,
experimental,
signal: renderSignal,
Expand All @@ -333,7 +329,6 @@ export class WidgetBinding<T extends ModelState = ModelState> {
once: true,
});
}
this.#replayState(registrations, renderSignal);
}

#trackCleanup(cleanup: Cleanup, reason: string): Promise<void> {
Expand All @@ -343,31 +338,6 @@ export class WidgetBinding<T extends ModelState = ModelState> {
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.
Expand Down
Loading