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.
2026-05-18 17:44:27 +00:00
parent 27c1f2671b
commit 32b2e20c54
3 changed files with 315 additions and 36 deletions
--- 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<string, unknown>): Record<string, unknown> {
+  const clean: Record<string, unknown> = {};
+  for (const key of Object.keys(props)) {
+    if (typeof props[key] !== "function") {
+      clean[key] = props[key];
+    }
+  }
+  return clean;
+}
+
+function cloneProps(props: Record<string, unknown>): Record<string, unknown> {
+  try {
+    return Value.Clone(props);
+  } catch {
+    const clone: Record<string, unknown> = {};
+    for (const key of Object.keys(props)) {
+      clone[key] = props[key];
+    }
+    return clone;
+  }
+}
+
 export interface CommitContext<I> {
  host: HostConfig<string, I, unknown>;
  ctx: unknown;
@@ -71,8 +93,12 @@ export function reconcileProps<I>(
    }
  }

-  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<I>(
        );
        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<I>(
  if (fiber.tag !== "#text" && fiber.tag !== el.type) return;

  const nextProps = el.props as Record<string, unknown>;
-  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<I>(

  const movedSet = new Set<Fiber<I>>(classification.moves.values());

-  // Map matched index (position in classification.matched[]) for quick lookup
  const matchedArrayIndex = new Map<MatchedChild<I>, 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<number, MatchedChild<I>>();
  for (const m of classification.matched) {
    matchedByNewIndex.set(m.index, m);
@@ -385,25 +432,21 @@ export function commitMutations<I>(
    addedByIndex.set(index, newChild);
  }

-  // Collect all positions in new-children order
  const allIndices = new Set<number>();
  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<I, unknown>, ctx);
  }

-  // Build new children array and determine placement actions
  const newChildren: Fiber<I>[] = [];
  type Placement = { fiber: Fiber<I>; beforeFiber: Fiber<I> | null; kind: "insert" | "move" };
  const placements: Placement[] = [];

-  // Create fibers for added children first (so they're available for before-lookup)
  const addedFibers = new Map<number, Fiber<I>>();
  for (const { index } of classification.added) {
    const newChild = addedByIndex.get(index)!;
@@ -411,13 +454,11 @@ export function commitMutations<I>(
    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<I>(
    }
  }

-  // 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<I>(
    }
  }

-  // 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<string, I, unknown>, ctx);
  commitHashes(parentFiber);
 }
--- a/test/value-diff-payloads.test.ts
+++ 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<string, unknown>;
+    nextProps: Record<string, unknown>;
+  }[] = [];
+  const prepareUpdateCalls: {
+    instance: string;
+    tag: string;
+    prevProps: Record<string, unknown>;
+    nextProps: Record<string, unknown>;
+  }[] = [];
+
+  const host: HostConfig<string, string, Record<string, unknown>> = {
+    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<string, unknown> = {};
+      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<string, string, Record<string, unknown>> = {
+        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<string, string, unknown>,
+        {},
+      );
+
+      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");
+    });
+  });
+});
--- 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);
    });