From 3f6ae8691ed483f8361b768d4084c20fe833163a Mon Sep 17 00:00:00 2001 From: "glm-5.1" Date: Mon, 18 May 2026 17:29:56 +0000 Subject: [PATCH] Use Value.Clone for deep prevProps snapshots before mutation --- src/host/reconcile.ts | 4 +- test/value-clone-prevprops.test.ts | 152 +++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 test/value-clone-prevprops.test.ts diff --git a/src/host/reconcile.ts b/src/host/reconcile.ts index c2c3ea0..bfc8f8e 100644 --- a/src/host/reconcile.ts +++ b/src/host/reconcile.ts @@ -78,7 +78,7 @@ export function reconcileProps( ); if (payload !== null && payload !== undefined) { fiber.effect = { type: "update", payload }; - fiber.prevProps = { ...fiber.props }; + fiber.prevProps = Value.Clone(fiber.props); fiber.props = nextProps; } } @@ -120,7 +120,7 @@ export function reconcileProps( if (payload !== null && payload !== undefined) { fiber.effect = { type: "update", payload }; - fiber.prevProps = { ...fiber.props }; + fiber.prevProps = Value.Clone(fiber.props); fiber.props = nextProps; } diff --git a/test/value-clone-prevprops.test.ts b/test/value-clone-prevprops.test.ts new file mode 100644 index 0000000..402e40d --- /dev/null +++ b/test/value-clone-prevprops.test.ts @@ -0,0 +1,152 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { Value } from "@alkdev/typebox/value"; +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 makeTrackingHost() { + const commitUpdateCalls: { + instance: string; + payload: unknown; + tag: string; + prevProps: Record; + nextProps: Record; + }[] = []; + + const host: HostConfig> = { + name: "tracking", + createRootContext: () => ({}), + createInstance: (tag, _props) => `${tag}_inst`, + createTextInstance: (text) => `text:${text}`, + appendChild: () => {}, + prepareUpdate: (_instance, _tag, prevProps, 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; + } + } + 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 }; +} + +describe("Value.Clone for prevProps snapshots", () => { + beforeEach(() => { + resetUpdateQueue(); + }); + + it("mutating fiber.props after setting prevProps does not affect prevProps", () => { + const { host } = makeTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { style: { color: "red", size: 10 } })); + + const divFiber = root.rootFiber!.children[0]!; + + reconcileProps( + divFiber, + h("div", { style: { color: "blue", size: 20 } }), + host as HostConfig, + {}, + ); + commitEffects(divFiber, host as HostConfig, {}); + + expect(divFiber.prevProps).not.toBeNull(); + const prevStyle = (divFiber.prevProps!.style as Record); + + (divFiber.props.style as Record).color = "green"; + + expect(prevStyle.color).toBe("red"); + expect((divFiber.prevProps!.style as Record).color).toBe("red"); + }); + + it("commitUpdate receives independent prevProps and nextProps with no shared references", () => { + const { host, commitUpdateCalls } = makeTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { style: { color: "red" } })); + + commitUpdateCalls.length = 0; + + root.render(h("div", { style: { color: "blue" } })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + expect(divCall).toBeDefined(); + + const prevStyle = divCall!.prevProps.style as Record; + const nextStyle = divCall!.nextProps.style as Record; + + expect(prevStyle.color).toBe("red"); + expect(nextStyle.color).toBe("blue"); + }); + + it("prevProps is a deep clone — nested objects are independent", () => { + const { host } = makeTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { data: { nested: { deep: "original" } } })); + + const divFiber = root.rootFiber!.children[0]!; + reconcileProps( + divFiber, + h("div", { data: { nested: { deep: "changed" } } }), + host as HostConfig, + {}, + ); + + expect(divFiber.prevProps).not.toBeNull(); + const prevNested = (divFiber.prevProps!.data as Record).nested as Record; + expect(prevNested.deep).toBe("original"); + }); + + it("commitUpdate receives correct before/after prop values", () => { + const { host, commitUpdateCalls } = makeTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", { color: "red", count: 1 })); + + commitUpdateCalls.length = 0; + + root.render(h("div", { color: "blue", count: 2 })); + + const divCall = commitUpdateCalls.find((c) => c.tag === "div"); + expect(divCall).toBeDefined(); + expect(divCall!.prevProps.color).toBe("red"); + expect(divCall!.nextProps.color).toBe("blue"); + expect(divCall!.prevProps.count).toBe(1); + expect(divCall!.nextProps.count).toBe(2); + }); + + it("text node prevProps is deep cloned", () => { + const { host } = makeTrackingHost(); + const root = createHostRoot(host, {}); + root.render(h("div", null, "hello")); + + const divFiber = root.rootFiber!.children[0]!; + const textFiber = divFiber.children[0]!; + + reconcileProps( + textFiber, + "world", + host as HostConfig, + {}, + ); + commitEffects(textFiber, host as HostConfig, {}); + + expect(textFiber.prevProps).not.toBeNull(); + expect(textFiber.prevProps!.text).toBe("hello"); + expect(textFiber.props.text).toBe("world"); + }); +}); \ No newline at end of file