From 080d7f79fd55d5bd0c2e8b46c198775e8d9b7a72 Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Thu, 21 May 2026 22:04:35 +0000 Subject: [PATCH 1/5] tree: lazily allocate KernelEventBuffer on first events access --- .../src/simple-tree/core/treeNodeKernel.ts | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts index fdb26e271d15..986ec98192be 100644 --- a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts +++ b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts @@ -121,13 +121,13 @@ export class TreeNodeKernel { /** * Events registered before hydration. + * * @remarks - * Since these are usually not used, they are allocated lazily as an optimization. - * The laziness also avoids extra forwarding overhead for events from this kernel's anchor node and also avoids registering for events that are unneeded. - * This means optimizations like skipping processing data in subtrees where no subtreeChanged events are subscribed to would be able to work, - * since the kernel does not unconditionally subscribe to those events (like a design which simply forwards all events would). + * Allocated lazily on first access to {@link TreeNodeKernel.events}. + * We expect the majority of nodes to never have event listeners registered, so + * deferring construction avoids per-kernel allocations. */ - readonly #eventBuffer: KernelEventBuffer; + #eventBuffer: KernelEventBuffer | undefined; /** * Create a TreeNodeKernel which can be looked up with {@link getKernel}. @@ -157,12 +157,9 @@ export class TreeNodeKernel { this.#hydrationState = { innerNode, }; - - this.#eventBuffer = new KernelEventBuffer(innerNode.events); } else { // Hydrated case this.#hydrationState = this.createHydratedState(innerNode); - this.#eventBuffer = new KernelEventBuffer(innerNode.anchorNode.events); } } @@ -190,8 +187,10 @@ export class TreeNodeKernel { this.#hydrationState = this.createHydratedState(inner); - // Lazily migrate existing event listeners to the anchor node - this.#eventBuffer.migrateEventSource(inner.anchorNode.events); + // Lazily migrate existing event listeners to the anchor node. + // If no one ever subscribed to this kernel's events, the buffer was never allocated + // and there is nothing to migrate. + this.#eventBuffer?.migrateEventSource(inner.anchorNode.events); } private createHydratedState(innerNode: HydratedFlexTreeNode): HydratedState { @@ -233,6 +232,13 @@ export class TreeNodeKernel { } public get events(): Listenable { + // Allocate the buffer on first access. See {@link TreeNodeKernel.#eventBuffer} for rationale. + if (this.#eventBuffer === undefined) { + const eventSource = isHydrated(this.#hydrationState) + ? this.#hydrationState.innerNode.anchorNode.events + : this.#hydrationState.innerNode.events; + this.#eventBuffer = new KernelEventBuffer(eventSource); + } return this.#eventBuffer; } @@ -244,7 +250,7 @@ export class TreeNodeKernel { off(); } } - this.#eventBuffer.dispose(); + this.#eventBuffer?.dispose(); // TODO: go to the context and remove myself from withAnchors } From 96efd43c63ce7f8279439d1cae9b074f6924bb0c Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Thu, 21 May 2026 22:34:07 +0000 Subject: [PATCH 2/5] test(tree): cover lazy KernelEventBuffer allocation semantics --- .../src/simple-tree/core/treeNodeKernel.ts | 1 + .../simple-tree/core/treeNodeKernel.spec.ts | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts index 986ec98192be..e1df7eeb8eb8 100644 --- a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts +++ b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts @@ -546,6 +546,7 @@ class KernelEventBuffer implements Listenable { } public on(eventName: keyof KernelEvents, listener: KernelEvents[typeof eventName]): Off { + assert(!this.#disposed, "Cannot register events on a disposed node"); // Lazily bind event listeners to the source. // If we do not have any existing listeners for this event, then we need to bind to the source. if (!this.#events.hasListeners(eventName)) { diff --git a/packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts b/packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts index 75f4afab82b0..3961df5e9788 100644 --- a/packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts @@ -93,6 +93,42 @@ describe("simple-tree proxies", () => { assert.equal(anchors.find(path), undefined); assert(anchors.isEmpty()); }); + + it("can hydrate a node with existing event listeners", () => { + // Listeners registered before hydration must continue to fire after the + // kernel migrates its event source from the unhydrated inner node to the + // hydrated anchor node. + const node = new ChildSchema({ content: 1 }); + + const log: string[] = []; + TreeBeta.on(node, "nodeChanged", ({ changedProperties }) => { + log.push(`nodeChanged: ${JSON.stringify([...changedProperties.keys()].sort())}`); + }); + + // Mutating before hydration: listener fires from the unhydrated event source. + node.content = 2; + assert.deepEqual(log, ['nodeChanged: ["content"]']); + + hydrate(ChildSchema, node); + + // Mutating after hydration: listener must continue firing, now from the anchor-node source. + node.content = 3; + assert.deepEqual(log, ['nodeChanged: ["content"]', 'nodeChanged: ["content"]']); + }); + + it("registering event listeners on a disposed kernel throws", () => { + // Once a kernel has been disposed (e.g. via afterDestroy on its anchor node), + // accessing its events to subscribe must fail loudly rather than silently + // allocating a fresh buffer on a defunct kernel. + const node = new ChildSchema({ content: 1 }); + hydrate(ChildSchema, node); + getKernel(node).dispose(); + + assert.throws( + () => TreeBeta.on(node, "nodeChanged", () => {}), + /Cannot register events on a disposed node/, + ); + }); }); describe("withBufferedTreeEvents", () => { @@ -151,6 +187,22 @@ describe("withBufferedTreeEvents", () => { assert.equal(eventCounter, 1); // Only a single event should have been raised. }); + it("subscribe then unsubscribe inside a scope yields emits no events", () => { + // A buffer that's been allocated lazily, then drained of listeners before the + // scope closes, must pop cleanly without emitting anything. + const obj = hydrate(MyObject, new MyObject({ foo: "init", bar: true })); + const log: string[] = []; + + withBufferedTreeEvents(() => { + const off = TreeBeta.on(obj, "nodeChanged", ({ changedProperties }) => { + log.push(`nodeChanged: ${JSON.stringify([...changedProperties.keys()].sort())}`); + }); + obj.foo = "outer"; + off(); + }); + assert.deepEqual(log, []); + }); + describe("nested calls", () => { // Helper: build a fresh logged subject for each test. function makeSubject(): { From 8e05fb3944c26f9443c889f133f6edd61a57bd5c Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Thu, 21 May 2026 15:51:52 -0700 Subject: [PATCH 3/5] test: Fix typo in test name Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts b/packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts index 3961df5e9788..ad61f668353e 100644 --- a/packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts @@ -187,7 +187,7 @@ describe("withBufferedTreeEvents", () => { assert.equal(eventCounter, 1); // Only a single event should have been raised. }); - it("subscribe then unsubscribe inside a scope yields emits no events", () => { + it("subscribe then unsubscribe inside a scope emits no events", () => { // A buffer that's been allocated lazily, then drained of listeners before the // scope closes, must pop cleanly without emitting anything. const obj = hydrate(MyObject, new MyObject({ foo: "init", bar: true })); From 3514df0acb4d85c21d9aa4e657de6bbdc608a220 Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Thu, 21 May 2026 22:55:15 +0000 Subject: [PATCH 4/5] docs: Update comment --- packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts index e1df7eeb8eb8..bddcb39c32a9 100644 --- a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts +++ b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts @@ -120,9 +120,11 @@ export class TreeNodeKernel { #hydrationState: HydrationState; /** - * Events registered before hydration. + * Handler for events listeners registered with the kernel. * * @remarks + * Supports event buffering via {@link withBufferedEvents}. + * * Allocated lazily on first access to {@link TreeNodeKernel.events}. * We expect the majority of nodes to never have event listeners registered, so * deferring construction avoids per-kernel allocations. @@ -547,6 +549,7 @@ class KernelEventBuffer implements Listenable { public on(eventName: keyof KernelEvents, listener: KernelEvents[typeof eventName]): Off { assert(!this.#disposed, "Cannot register events on a disposed node"); + // Lazily bind event listeners to the source. // If we do not have any existing listeners for this event, then we need to bind to the source. if (!this.#events.hasListeners(eventName)) { From 61698582d9f19c6bf146d431f7e8805d14ea689a Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Thu, 21 May 2026 22:59:29 +0000 Subject: [PATCH 5/5] improvement: More consistent assertions --- packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts index bddcb39c32a9..1cf29211dc11 100644 --- a/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts +++ b/packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts @@ -234,6 +234,7 @@ export class TreeNodeKernel { } public get events(): Listenable { + assert(!this.disposed, "Cannot register events on a disposed node"); // Allocate the buffer on first access. See {@link TreeNodeKernel.#eventBuffer} for rationale. if (this.#eventBuffer === undefined) { const eventSource = isHydrated(this.#hydrationState) @@ -548,7 +549,7 @@ class KernelEventBuffer implements Listenable { } public on(eventName: keyof KernelEvents, listener: KernelEvents[typeof eventName]): Off { - assert(!this.#disposed, "Cannot register events on a disposed node"); + this.#assertNotDisposed(); // Lazily bind event listeners to the source. // If we do not have any existing listeners for this event, then we need to bind to the source.