diff --git a/packages/shared/src/Struct.edgeCases.test.ts b/packages/shared/src/Struct.edgeCases.test.ts new file mode 100644 index 00000000000..047f3628450 --- /dev/null +++ b/packages/shared/src/Struct.edgeCases.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vite-plus/test"; + +import type { DeepPartial } from "./Struct.ts"; +import { deepMerge } from "./Struct.ts"; + +// This suite pins the SURPRISING / LOSSY runtime contract of `deepMerge`. +// It complements Struct.test.ts and must never assert behavior the function +// does not have — only what it ACTUALLY does today, so regressions are caught. +// +// Key detail: `deepMerge` recurses only when BOTH sides are plain objects. +// In effect v4 (the installed version), `Predicate.isObject` is +// `typeof v === "object" && v !== null && !Array.isArray(v)`, so it returns +// `true` for plain objects, `Date`, `Map`, and class instances, but `false` +// for arrays, functions, `null`, `undefined`, and primitives. +describe("deepMerge edge cases", () => { + it("overwrites arrays instead of concatenating them", () => { + // Intentional quirk documentation: arrays are NOT plain objects, so the + // recursion branch is skipped and the patch array is assigned directly + // (whole-value overwrite). + const base = { tags: ["a", "b"] }; + const patch = { tags: ["c"] }; + const result = deepMerge(base, patch); + expect(result).toEqual({ tags: ["c"] }); + expect(Array.isArray(result.tags)).toBe(true); + expect(result.tags).toEqual(["c"]); + }); + + // KNOWN LIMITATION: deepMerge only skips `undefined` (value === undefined). + // JSON/null-able sources can silently overwrite with null. Consider changing + // the guard to value == null if skip-null is desired. + it("[LIMITATION] null is not skipped (only undefined is)", () => { + const base = { a: 1 }; + const patch = { a: null } as unknown as DeepPartial<{ a: number }>; + const result = deepMerge(base, patch); + expect(result).toEqual({ a: null }); + expect(result.a).toBe(null); + }); + + it("overwrites Date values whole, preserving the prototype (regression: deepMerge-date-prototype-loss)", () => { + // @effect-diagnostics-next-line globalDate:off + const base = { d: new Date(0) }; + // @effect-diagnostics-next-line globalDate:off + const patch = { d: new Date(1000) }; + const result = deepMerge(base, patch); + expect(result.d).toBeInstanceOf(Date); + expect((result.d as Date).getTime()).toBe(1000); + }); + + it("overwrites function values with the patch function", () => { + const base = { fn: () => 1 }; + const second = () => 2; + const patch = { fn: second } as unknown as DeepPartial<{ fn: () => number }>; + const result = deepMerge(base, patch); + expect(result.fn).toBe(second); + expect(result.fn()).toBe(2); + }); + + it("skips nested undefined patch keys (recurses)", () => { + type Shape = { a: number; nested: { x: number; y: number } }; + const base: Shape = { a: 1, nested: { x: 1, y: 2 } }; + const withNestedUndefined = { y: undefined } as unknown as DeepPartial; + const patch = { nested: withNestedUndefined } as unknown as DeepPartial; + const result = deepMerge(base, patch); + expect(result).toEqual({ a: 1, nested: { x: 1, y: 2 } }); + }); + + it("throws on a primitive patch instead of discarding the base (regression: deepMerge-primitive-discards-base)", () => { + const base = { a: 1, b: 2 }; + const patch = 5 as unknown as DeepPartial<{ a: number; b: number }>; + expect(() => deepMerge(base, patch)).toThrow(); + }); +}); diff --git a/packages/shared/src/Struct.test.ts b/packages/shared/src/Struct.test.ts new file mode 100644 index 00000000000..5cb4df58155 --- /dev/null +++ b/packages/shared/src/Struct.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { deepMerge, type DeepPartial } from "./Struct.ts"; + +describe("deepMerge", () => { + it("performs a shallow merge of two flat objects", () => { + const base = { a: 1, b: 2 }; + const patch = { b: 3, c: 4 }; + expect(deepMerge(base, patch)).toEqual({ a: 1, b: 3, c: 4 }); + }); + + it("does not mutate the base object", () => { + const base = { a: 1, b: 2 }; + const snapshot = { ...base }; + deepMerge(base, { b: 9 }); + expect(base).toEqual(snapshot); + }); + + it("recursively merges nested objects", () => { + const base = { user: { name: "alice", age: 30 }, active: true }; + const patch = { user: { age: 31 } }; + expect(deepMerge(base, patch)).toEqual({ + user: { name: "alice", age: 31 }, + active: true, + }); + expect(base.user).toEqual({ name: "alice", age: 30 }); + }); + + it("merges multiple patch objects in sequence", () => { + const base = { a: 1, nested: { x: 1, y: 2 } }; + const merged = deepMerge(deepMerge(base, { a: 2 }), { nested: { y: 9 } }); + expect(merged).toEqual({ a: 2, nested: { x: 1, y: 9 } }); + expect(base.nested).toEqual({ x: 1, y: 2 }); + }); + + it("skips keys with undefined values in the patch", () => { + const base = { a: 1, b: 2 }; + const undefinedA: unknown = undefined; + const patch = { a: undefinedA, b: 5 } as DeepPartial; + expect(deepMerge(base, patch)).toEqual({ a: 1, b: 5 }); + }); + + it("returns a shallow copy of the base (nested refs shared — known limitation)", () => { + const base = { a: 1, b: { c: 2 } }; + const merged = deepMerge(base, {}); + expect(merged).toEqual({ a: 1, b: { c: 2 } }); + // DOCUMENTED LIMITATION (not ideal): deepMerge only shallow-copies the top + // level. Nested objects NOT touched by the patch keep the SAME reference as + // base, so mutating `merged.b` would mutate `base.b`. A future fix should + // deep-clone untouched nested objects. Do not "fix" this test by changing + // the assertion to `not.toBe` until that fix lands. + expect(merged).not.toBe(base); + expect(merged.b).toBe(base.b); + }); + + it("handles an empty base with a populated patch", () => { + const base = {} as Record; + const patch = { a: 1, nested: { b: 2 } }; + expect(deepMerge(base, patch)).toEqual({ a: 1, nested: { b: 2 } }); + }); +}); diff --git a/packages/shared/src/Struct.ts b/packages/shared/src/Struct.ts index f703bcabfaa..23057b8e0f2 100644 --- a/packages/shared/src/Struct.ts +++ b/packages/shared/src/Struct.ts @@ -6,8 +6,24 @@ export type DeepPartial = T extends readonly (infer U)[] ? { [K in keyof T]?: DeepPartial } : T; +function isPlainObject(value: unknown): value is Record { + return ( + value !== null && + typeof value === "object" && + !Array.isArray(value) && + Object.getPrototypeOf(value) === Object.prototype + ); +} + export function deepMerge>(current: T, patch: DeepPartial): T { - if (!P.isObject(current) || !P.isObject(patch)) { + if (!P.isObject(current)) { + return patch as T; + } + + if (!P.isObject(patch)) { + if (patch !== null) { + throw new Error(`deepMerge: patch must be a plain object, received ${typeof patch}`); + } return patch as T; } @@ -16,7 +32,8 @@ export function deepMerge>(current: T, patch: if (value === undefined) continue; const existing = next[key]; - next[key] = P.isObject(existing) && P.isObject(value) ? deepMerge(existing, value) : value; + next[key] = + isPlainObject(existing) && isPlainObject(value) ? deepMerge(existing, value) : value; } return next as T;