From 32b2e20c54414f3db95971877ce9afdce7212c28 Mon Sep 17 00:00:00 2001 From: "glm-5.1" Date: Mon, 18 May 2026 17:44:27 +0000 Subject: [PATCH] feat: add Value.Diff granular prop payloads for commitUpdate Value.Diff produces property-level diff payloads instead of relying solely on prepareUpdate. Function-valued props are stripped before diffing; ValueDiffError is caught and falls back to prepareUpdate. Value.Equal and Value.Clone now safely handle function-valued props with try-catch fallbacks. --- src/host/reconcile.ts | 84 +++++++--- test/value-diff-payloads.test.ts | 244 ++++++++++++++++++++++++++++++ test/value-hash-detection.test.ts | 23 ++- 3 files changed, 315 insertions(+), 36 deletions(-) create mode 100644 test/value-diff-payloads.test.ts diff --git a/src/host/reconcile.ts b/src/host/reconcile.ts index cc85711..288a486 100644 --- a/src/host/reconcile.ts +++ b/src/host/reconcile.ts @@ -1,11 +1,33 @@ import { effect } from "@preact/signals-core"; -import { Value } from "@alkdev/typebox/value"; +import { Value, ValueDiffError } from "@alkdev/typebox/value"; import type { Fiber, Effect, HostLike } from "./fiber.js"; import { disposeFiber } from "./fiber.js"; import type { HostConfig } from "./config.js"; import type { UNode, UElement } from "../core/schema.js"; import { isUPrimitive, isURoot, isUElement } from "../core/schema.js"; +function stripFunctionProps(props: Record): Record { + const clean: Record = {}; + for (const key of Object.keys(props)) { + if (typeof props[key] !== "function") { + clean[key] = props[key]; + } + } + return clean; +} + +function cloneProps(props: Record): Record { + try { + return Value.Clone(props); + } catch { + const clone: Record = {}; + for (const key of Object.keys(props)) { + clone[key] = props[key]; + } + return clone; + } +} + export interface CommitContext { host: HostConfig; ctx: unknown; @@ -71,8 +93,12 @@ export function reconcileProps( } } - if (fiber.cachedNode !== null && Value.Equal(fiber.cachedNode, nextNode)) { - return; + if (fiber.cachedNode !== null) { + try { + if (Value.Equal(fiber.cachedNode, nextNode)) return; + } catch { + // Value.Equal may throw on function values; fall through + } } if (isUPrimitive(nextNode)) { @@ -89,7 +115,7 @@ export function reconcileProps( ); if (payload !== null && payload !== undefined) { fiber.effect = { type: "update", payload }; - fiber.prevProps = Value.Clone(fiber.props); + fiber.prevProps = cloneProps(fiber.props); fiber.props = nextProps; } } @@ -121,17 +147,40 @@ export function reconcileProps( if (fiber.tag !== "#text" && fiber.tag !== el.type) return; const nextProps = el.props as Record; - const payload = host.prepareUpdate?.( - fiber.instance, - fiber.tag as never, - fiber.props, - nextProps, - ctx as never, - ); + let payload: unknown; + + try { + const cleanPrev = stripFunctionProps(fiber.props); + const cleanNext = stripFunctionProps(nextProps); + const diff = Value.Diff(cleanPrev, cleanNext); + if (diff.length > 0) { + payload = diff; + } else { + payload = host.prepareUpdate?.( + fiber.instance, + fiber.tag as never, + fiber.props, + nextProps, + ctx as never, + ); + } + } catch (e: unknown) { + if (e instanceof ValueDiffError || e instanceof Error) { + payload = host.prepareUpdate?.( + fiber.instance, + fiber.tag as never, + fiber.props, + nextProps, + ctx as never, + ); + } else { + throw e; + } + } if (payload !== null && payload !== undefined) { fiber.effect = { type: "update", payload }; - fiber.prevProps = Value.Clone(fiber.props); + fiber.prevProps = cloneProps(fiber.props); fiber.props = nextProps; } @@ -369,13 +418,11 @@ export function commitMutations( const movedSet = new Set>(classification.moves.values()); - // Map matched index (position in classification.matched[]) for quick lookup const matchedArrayIndex = new Map, number>(); for (let i = 0; i < classification.matched.length; i++) { matchedArrayIndex.set(classification.matched[i]!, i); } - // Build maps keyed by new-children position const matchedByNewIndex = new Map>(); for (const m of classification.matched) { matchedByNewIndex.set(m.index, m); @@ -385,25 +432,21 @@ export function commitMutations( addedByIndex.set(index, newChild); } - // Collect all positions in new-children order const allIndices = new Set(); for (const m of classification.matched) allIndices.add(m.index); for (const a of classification.added) allIndices.add(a.index); const sortedIndices = [...allIndices].sort((a, b) => a - b); - // Phase 1: Removes — reverse order (children before parents, bottom-up) for (let i = classification.removed.length - 1; i >= 0; i--) { const fiber = classification.removed[i]!; host.removeChild?.(parentInst as never, fiber.instance as never, ctx as never); disposeFiber(fiber, host as HostLike, ctx); } - // Build new children array and determine placement actions const newChildren: Fiber[] = []; type Placement = { fiber: Fiber; beforeFiber: Fiber | null; kind: "insert" | "move" }; const placements: Placement[] = []; - // Create fibers for added children first (so they're available for before-lookup) const addedFibers = new Map>(); for (const { index } of classification.added) { const newChild = addedByIndex.get(index)!; @@ -411,13 +454,11 @@ export function commitMutations( addedFibers.set(index, fiber); } - // Build the final children array for (const idx of sortedIndices) { const addedChild = addedByIndex.get(idx); if (addedChild !== undefined) { const fiber = addedFibers.get(idx)!; newChildren.push(fiber); - // Find before: next fiber in newChildren that is "staying" (not moved, not just inserted) const before = findNextStayingFiber(sortedIndices, matchedByNewIndex, addedByIndex, movedSet, idx); placements.push({ fiber, beforeFiber: before, kind: "insert" }); continue; @@ -435,7 +476,6 @@ export function commitMutations( } } - // Phase 2: Inserts + Moves — left-to-right for (const { fiber, beforeFiber } of placements) { if (host.insertBefore && beforeFiber) { host.insertBefore( @@ -449,13 +489,11 @@ export function commitMutations( } } - // Update fiber tree parentFiber.children = newChildren; for (const fiber of classification.removed) { fiber.parent = null; } - // Phase 3: Updates — top-down (parent before child) via commitEffects commitEffects(parentFiber, host as HostConfig, ctx); commitHashes(parentFiber); } diff --git a/test/value-diff-payloads.test.ts b/test/value-diff-payloads.test.ts new file mode 100644 index 0000000..9e4168a --- /dev/null +++ b/test/value-diff-payloads.test.ts @@ -0,0 +1,244 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { h } from "../src/core/h.js"; +import { createRoot as createHostRoot } from "../src/host/config.js"; +import type { HostConfig } from "../src/host/config.js"; +import { reconcileProps, commitEffects, resetUpdateQueue } from "../src/host/reconcile.js"; +import type { Fiber } from "../src/host/fiber.js"; + +function makeDiffTrackingHost() { + const commitUpdateCalls: { + instance: string; + payload: unknown; + tag: string; + prevProps: Record; + nextProps: Record; + }[] = []; + const prepareUpdateCalls: { + instance: string; + tag: string; + prevProps: Record; + nextProps: Record; + }[] = []; + + const host: HostConfig> = { + name: "diff-tracking", + createRootContext: () => ({}), + createInstance: (tag, _props) => `${tag}_inst`, + createTextInstance: (text) => `text:${text}`, + appendChild: () => {}, + prepareUpdate: (_instance, tag, prevProps, nextProps) => { + prepareUpdateCalls.push({ instance: _instance, tag, prevProps: { ...prevProps }, nextProps: { ...nextProps } }); + const changed: Record = {}; + let hasChanges = false; + for (const key of Object.keys(nextProps)) { + if (prevProps[key] !== nextProps[key]) { + changed[key] = nextProps[key]; + hasChanges = true; + } + } + for (const key of Object.keys(prevProps)) { + if (!(key in nextProps)) { + changed[key] = undefined; + hasChanges = true; + } + } + return hasChanges ? changed : null; + }, + commitUpdate: (instance, payload, tag, prevProps, nextProps) => { + commitUpdateCalls.push({ + instance: instance as string, + payload, + tag: tag as string, + prevProps, + nextProps, + }); + }, + }; + + return { host, commitUpdateCalls, prepareUpdateCalls }; +} + +describe("Value.Diff granular prop payloads", () => { + beforeEach(() => { + resetUpdateQueue(); + }); + + describe("commitUpdate receives Value.Diff payload with only changed keys", () => { + it("diff payload contains only changed props", () => { + const { host, commitUpdateCalls } = makeDiffTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { color: "red", size: 10 })); + + commitUpdateCalls.length = 0; + + root.render(h("div", { color: "blue", size: 10 })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + expect(divCall).toBeDefined(); + + const payload = divCall!.payload as Array<{ type: string; path: string; value: unknown }>; + expect(Array.isArray(payload)).toBe(true); + expect(payload.length).toBe(1); + expect(payload[0]!.type).toBe("update"); + expect(payload[0]!.path).toBe("/color"); + expect(payload[0]!.value).toBe("blue"); + }); + + it("diff payload with multiple changed props", () => { + const { host, commitUpdateCalls } = makeDiffTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { color: "red", size: 10, label: "a" })); + + commitUpdateCalls.length = 0; + + root.render(h("div", { color: "blue", size: 20, label: "a" })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + expect(divCall).toBeDefined(); + + const payload = divCall!.payload as Array<{ type: string; path: string; value: unknown }>; + expect(Array.isArray(payload)).toBe(true); + const paths = payload.map((d) => d.path).sort(); + expect(paths).toEqual(["/color", "/size"]); + }); + + it("no changes produces no commitUpdate", () => { + const { host, commitUpdateCalls } = makeDiffTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { color: "red" })); + + commitUpdateCalls.length = 0; + + root.render(h("div", { color: "red" })); + + expect(commitUpdateCalls.length).toBe(0); + }); + + it("added prop appears as insert in diff payload", () => { + const { host, commitUpdateCalls } = makeDiffTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { color: "red" })); + + commitUpdateCalls.length = 0; + + root.render(h("div", { color: "red", size: 10 })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + expect(divCall).toBeDefined(); + + const payload = divCall!.payload as Array<{ type: string; path: string; value: unknown }>; + expect(Array.isArray(payload)).toBe(true); + expect(payload.some((d) => d.type === "insert" && d.path === "/size")).toBe(true); + }); + + it("removed prop appears as delete in diff payload", () => { + const { host, commitUpdateCalls } = makeDiffTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { color: "red", size: 10 })); + + commitUpdateCalls.length = 0; + + root.render(h("div", { color: "red" })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + expect(divCall).toBeDefined(); + + const payload = divCall!.payload as Array<{ type: string; path: string; value: unknown }>; + expect(Array.isArray(payload)).toBe(true); + expect(payload.some((d) => d.type === "delete" && d.path === "/size")).toBe(true); + }); + }); + + describe("function-valued props don't crash Value.Diff (stripped or caught)", () => { + it("element with function props still updates correctly", () => { + const { host, commitUpdateCalls } = makeDiffTrackingHost(); + const root = createHostRoot(host, {}); + const onClick = () => {}; + root.render(h("div", { color: "red", onClick })); + + commitUpdateCalls.length = 0; + + const onClick2 = () => {}; + root.render(h("div", { color: "blue", onClick: onClick2 })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + expect(divCall).toBeDefined(); + + const payload = divCall!.payload as Array<{ type: string; path: string; value: unknown }>; + expect(Array.isArray(payload)).toBe(true); + expect(payload.some((d) => d.path === "/color" && d.value === "blue")).toBe(true); + }); + + it("only function props changed — no diff entries for functions in Value.Diff payload", () => { + const { host, commitUpdateCalls } = makeDiffTrackingHost(); + const root = createHostRoot(host, {}); + const onClick1 = () => {}; + root.render(h("div", { color: "red", onClick: onClick1 })); + + commitUpdateCalls.length = 0; + + const onClick2 = () => {}; + root.render(h("div", { color: "red", onClick: onClick2 })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + if (divCall) { + const payload = divCall.payload; + if (Array.isArray(payload)) { + const functionPaths = (payload as Array<{ type: string; path: string; value: unknown }>).filter((d) => d.path === "/onClick"); + expect(functionPaths.length).toBe(0); + } + } + }); + }); + + describe("diff error falls back gracefully to prepareUpdate", () => { + it("ValueDiffError from nested function values falls back to prepareUpdate", () => { + let prepareUpdateCalled = false; + const host: HostConfig> = { + name: "fallback", + createRootContext: () => ({}), + createInstance: (tag) => `${tag}_inst`, + createTextInstance: (text) => `text:${text}`, + appendChild: () => {}, + prepareUpdate: (_instance, _tag, _prevProps, _nextProps) => { + prepareUpdateCalled = true; + return { fallback: true }; + }, + commitUpdate: () => {}, + }; + const root = createHostRoot(host, {}); + const deepFn = { handler: () => {} }; + root.render(h("div", { data: deepFn, color: "red" })); + + const divFiber = root.rootFiber!.children[0]!; + + const deepFn2 = { handler: () => {} }; + reconcileProps( + divFiber, + h("div", { data: deepFn2, color: "blue" }), + host as HostConfig, + {}, + ); + + expect(prepareUpdateCalled).toBe(true); + expect(divFiber.effect).not.toBeNull(); + }); + }); + + describe("hosts can still use prevProps/nextProps if they prefer", () => { + it("commitUpdate still receives prevProps and nextProps alongside diff payload", () => { + const { host, commitUpdateCalls } = makeDiffTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { color: "red" })); + + commitUpdateCalls.length = 0; + + root.render(h("div", { color: "blue" })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + expect(divCall).toBeDefined(); + expect(divCall!.prevProps.color).toBe("red"); + expect(divCall!.nextProps.color).toBe("blue"); + }); + }); +}); \ No newline at end of file diff --git a/test/value-hash-detection.test.ts b/test/value-hash-detection.test.ts index 4db3356..7397058 100644 --- a/test/value-hash-detection.test.ts +++ b/test/value-hash-detection.test.ts @@ -137,32 +137,32 @@ describe("Value.Hash O(1) change detection", () => { }); describe("hash mismatch falls through to Value.Equal / reconciliation", () => { - it("hash mismatch with changed prop triggers prepareUpdate", () => { - const { host, prepareUpdateCalls } = makeTrackingHost(); + it("hash mismatch with changed prop triggers commitUpdate", () => { + const { host, commitUpdateCalls } = makeTrackingHost(); const root = createHostRoot(host, {}); root.render(h("div", { color: "red" })); commitHashes(root.rootFiber!); - prepareUpdateCalls.length = 0; + commitUpdateCalls.length = 0; root.render(h("div", { color: "blue" })); - expect(prepareUpdateCalls.length).toBeGreaterThanOrEqual(1); + expect(commitUpdateCalls.length).toBeGreaterThanOrEqual(1); }); - it("hash mismatch for child triggers prepareUpdate for child only", () => { - const { host, prepareUpdateCalls } = makeTrackingHost(); + it("hash mismatch for child triggers commitUpdate for child only", () => { + const { host, commitUpdateCalls } = makeTrackingHost(); const root = createHostRoot(host, {}); root.render(h("div", { color: "red" }, h("span", { label: "a" }))); commitHashes(root.rootFiber!); - prepareUpdateCalls.length = 0; + commitUpdateCalls.length = 0; root.render(h("div", { color: "red" }, h("span", { label: "b" }))); - const spanCalls = prepareUpdateCalls.filter((c) => c.tag === "span"); + const spanCalls = commitUpdateCalls.filter((c) => c.tag === "span"); expect(spanCalls.length).toBe(1); expect(spanCalls[0]!.prevProps.label).toBe("a"); expect(spanCalls[0]!.nextProps.label).toBe("b"); @@ -171,7 +171,7 @@ describe("Value.Hash O(1) change detection", () => { describe("hash is computed outside reactive computations", () => { it("Value.Hash is never called from within a computed or effect callback", async () => { - const { host, prepareUpdateCalls, commitUpdateCalls } = makeTrackingHost(); + const { host, commitUpdateCalls } = makeTrackingHost(); const s = signal("red"); const root = createHostRoot(host, {}); @@ -187,7 +187,6 @@ describe("Value.Hash O(1) change detection", () => { {}, ); - prepareUpdateCalls.length = 0; commitUpdateCalls.length = 0; s.value = "blue"; @@ -196,13 +195,12 @@ describe("Value.Hash O(1) change detection", () => { queueMicrotask(() => resolve()); }); - expect(prepareUpdateCalls.length).toBeGreaterThanOrEqual(1); + expect(commitUpdateCalls.length).toBeGreaterThanOrEqual(1); expect(divFiber.props.color).toBe("blue"); const newHash = divFiber.hash; expect(newHash).not.toBeNull(); - prepareUpdateCalls.length = 0; commitUpdateCalls.length = 0; s.value = "blue"; @@ -211,7 +209,6 @@ describe("Value.Hash O(1) change detection", () => { queueMicrotask(() => resolve()); }); - expect(prepareUpdateCalls.length).toBe(0); expect(commitUpdateCalls.length).toBe(0); });