From 9e6015f60414a53115b32f1758f732ebed6725db Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 24 Jul 2025 12:54:32 -0700 Subject: [PATCH 1/2] [feat] Replace JSON.parse(JSON.stringify()) with structuredClone() for object cloning - Use modern structuredClone() API when available for deep cloning - Maintains fallback to JSON method for compatibility - Fixes circular reference crashes that previously threw errors - Preserves type fidelity (Date, RegExp, undefined values) - Performance neutral to positive due to native implementation - Add comprehensive test suite covering all edge cases Fixes #1157 --- src/LiteGraphGlobal.ts | 19 ++- test/LiteGraphGlobal.cloneObject.test.ts | 152 +++++++++++++++++++++++ 2 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 test/LiteGraphGlobal.cloneObject.test.ts diff --git a/src/LiteGraphGlobal.ts b/src/LiteGraphGlobal.ts index 72b2be01b..8bd4f79ae 100644 --- a/src/LiteGraphGlobal.ts +++ b/src/LiteGraphGlobal.ts @@ -614,7 +614,24 @@ export class LiteGraphGlobal { cloneObject(obj: T, target?: T): WhenNullish { if (obj == null) return null as WhenNullish - const r = JSON.parse(JSON.stringify(obj)) + let r: any + + // Use modern structuredClone when available (faster, handles circular refs, preserves types) + if (typeof structuredClone !== "undefined") { + try { + r = structuredClone(obj) + } catch (error) { + // Fallback for non-cloneable types (functions, DOM nodes, etc.) + if (this.debug) { + console.warn("structuredClone failed, falling back to JSON method:", error) + } + r = JSON.parse(JSON.stringify(obj)) + } + } else { + // Fallback for older environments + r = JSON.parse(JSON.stringify(obj)) + } + if (!target) return r for (const i in r) { diff --git a/test/LiteGraphGlobal.cloneObject.test.ts b/test/LiteGraphGlobal.cloneObject.test.ts new file mode 100644 index 000000000..614f891d1 --- /dev/null +++ b/test/LiteGraphGlobal.cloneObject.test.ts @@ -0,0 +1,152 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" + +import { LiteGraph } from "@/litegraph" + +describe("LiteGraph.cloneObject", () => { + beforeEach(() => { + // Reset debug state + LiteGraph.debug = false + }) + + it("should handle null and undefined", () => { + expect(LiteGraph.cloneObject(null)).toBe(null) + expect(LiteGraph.cloneObject()).toBe(null) + }) + + it("should clone simple objects", () => { + const obj = { a: 1, b: "test", c: true } + const cloned = LiteGraph.cloneObject(obj) + + expect(cloned).toEqual(obj) + expect(cloned).not.toBe(obj) + }) + + it("should preserve Date objects when structuredClone is available", () => { + const date = new Date("2024-01-01") + const obj = { timestamp: date } + + if (typeof structuredClone !== "undefined") { + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.timestamp).toBeInstanceOf(Date) + expect(cloned.timestamp.getTime()).toBe(date.getTime()) + } else { + // In environments without structuredClone, should fallback to JSON + const cloned = LiteGraph.cloneObject(obj) + expect(typeof cloned.timestamp).toBe("string") + } + }) + + it("should preserve RegExp objects when structuredClone is available", () => { + const regex = /test/gi + const obj = { pattern: regex } + + if (typeof structuredClone !== "undefined") { + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.pattern).toBeInstanceOf(RegExp) + expect(cloned.pattern.source).toBe("test") + expect(cloned.pattern.flags).toBe("gi") + } else { + // In environments without structuredClone, should fallback to JSON + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.pattern).toEqual({}) + } + }) + + it("should preserve undefined values when structuredClone is available", () => { + const obj = { defined: "value", undefined: undefined } + + if (typeof structuredClone !== "undefined") { + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.undefined).toBe(undefined) + expect("undefined" in cloned).toBe(true) + } else { + // JSON method converts undefined to null + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.undefined).toBe(null) + } + }) + + it("should handle circular references when structuredClone is available", () => { + const obj: any = { name: "test" } + obj.self = obj + + if (typeof structuredClone !== "undefined") { + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.name).toBe("test") + expect(cloned.self).toBe(cloned) // Circular reference preserved + expect(cloned).not.toBe(obj) // But it's still a clone + } else { + // Without structuredClone, should throw + expect(() => LiteGraph.cloneObject(obj)).toThrow() + } + }) + + it("should fallback to JSON method when structuredClone fails", () => { + // Mock structuredClone to throw an error + const originalStructuredClone = globalThis.structuredClone + const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + + try { + // @ts-expect-error - mocking global + globalThis.structuredClone = () => { + throw new Error("Mock structuredClone failure") + } + + LiteGraph.debug = true + const obj = { a: 1, b: "test" } + const cloned = LiteGraph.cloneObject(obj) + + expect(cloned).toEqual(obj) + expect(cloned).not.toBe(obj) + expect(consoleSpy).toHaveBeenCalledWith( + "structuredClone failed, falling back to JSON method:", + expect.any(Error), + ) + } finally { + globalThis.structuredClone = originalStructuredClone + consoleSpy.mockRestore() + } + }) + + it("should handle the target parameter correctly", () => { + const source = { a: 1, b: 2 } + const target = { c: 3 } + + const result = LiteGraph.cloneObject(source, target) + + expect(result).toBe(target) // Should return the target object + expect(result.a).toBe(1) + expect(result.b).toBe(2) + expect(result.c).toBe(3) // Original target property preserved + }) + + it("should clone typical node flags correctly", () => { + const flags = { + collapsed: false, + pinned: true, + selected: false, + } + + const cloned = LiteGraph.cloneObject(flags) + expect(cloned).toEqual(flags) + expect(cloned).not.toBe(flags) + }) + + it("should clone typical node properties correctly", () => { + const properties = { + value: 42, + name: "test node", + enabled: true, + nested: { + array: [1, 2, 3], + object: { x: 10, y: 20 }, + }, + } + + const cloned = LiteGraph.cloneObject(properties) + expect(cloned).toEqual(properties) + expect(cloned).not.toBe(properties) + expect(cloned.nested).not.toBe(properties.nested) + expect(cloned.nested.array).not.toBe(properties.nested.array) + }) +}) From 7f8dda0ca5ab503dfb3f4d7cb89382c594b20343 Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 24 Jul 2025 13:41:00 -0700 Subject: [PATCH 2/2] [refactor] Remove unnecessary fallback logic from cloneObject() Since structuredClone is already used extensively throughout the codebase (20+ places), there's no need for JSON fallback. Simplifies implementation and removes redundant error handling. - Removed try/catch fallback logic from cloneObject() - Simplified test suite to match streamlined implementation - All 277 tests passing Fixes #1157 --- src/LiteGraphGlobal.ts | 19 +---- test/LiteGraphGlobal.cloneObject.test.ts | 93 ++++++------------------ 2 files changed, 23 insertions(+), 89 deletions(-) diff --git a/src/LiteGraphGlobal.ts b/src/LiteGraphGlobal.ts index 8bd4f79ae..a784c488f 100644 --- a/src/LiteGraphGlobal.ts +++ b/src/LiteGraphGlobal.ts @@ -614,24 +614,7 @@ export class LiteGraphGlobal { cloneObject(obj: T, target?: T): WhenNullish { if (obj == null) return null as WhenNullish - let r: any - - // Use modern structuredClone when available (faster, handles circular refs, preserves types) - if (typeof structuredClone !== "undefined") { - try { - r = structuredClone(obj) - } catch (error) { - // Fallback for non-cloneable types (functions, DOM nodes, etc.) - if (this.debug) { - console.warn("structuredClone failed, falling back to JSON method:", error) - } - r = JSON.parse(JSON.stringify(obj)) - } - } else { - // Fallback for older environments - r = JSON.parse(JSON.stringify(obj)) - } - + const r = structuredClone(obj) if (!target) return r for (const i in r) { diff --git a/test/LiteGraphGlobal.cloneObject.test.ts b/test/LiteGraphGlobal.cloneObject.test.ts index 614f891d1..c065f29a8 100644 --- a/test/LiteGraphGlobal.cloneObject.test.ts +++ b/test/LiteGraphGlobal.cloneObject.test.ts @@ -21,92 +21,43 @@ describe("LiteGraph.cloneObject", () => { expect(cloned).not.toBe(obj) }) - it("should preserve Date objects when structuredClone is available", () => { + it("should preserve Date objects", () => { const date = new Date("2024-01-01") const obj = { timestamp: date } - - if (typeof structuredClone !== "undefined") { - const cloned = LiteGraph.cloneObject(obj) - expect(cloned.timestamp).toBeInstanceOf(Date) - expect(cloned.timestamp.getTime()).toBe(date.getTime()) - } else { - // In environments without structuredClone, should fallback to JSON - const cloned = LiteGraph.cloneObject(obj) - expect(typeof cloned.timestamp).toBe("string") - } + + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.timestamp).toBeInstanceOf(Date) + expect(cloned.timestamp.getTime()).toBe(date.getTime()) }) - it("should preserve RegExp objects when structuredClone is available", () => { + it("should preserve RegExp objects", () => { const regex = /test/gi const obj = { pattern: regex } - - if (typeof structuredClone !== "undefined") { - const cloned = LiteGraph.cloneObject(obj) - expect(cloned.pattern).toBeInstanceOf(RegExp) - expect(cloned.pattern.source).toBe("test") - expect(cloned.pattern.flags).toBe("gi") - } else { - // In environments without structuredClone, should fallback to JSON - const cloned = LiteGraph.cloneObject(obj) - expect(cloned.pattern).toEqual({}) - } + + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.pattern).toBeInstanceOf(RegExp) + expect(cloned.pattern.source).toBe("test") + expect(cloned.pattern.flags).toBe("gi") }) - it("should preserve undefined values when structuredClone is available", () => { + it("should preserve undefined values", () => { const obj = { defined: "value", undefined: undefined } - - if (typeof structuredClone !== "undefined") { - const cloned = LiteGraph.cloneObject(obj) - expect(cloned.undefined).toBe(undefined) - expect("undefined" in cloned).toBe(true) - } else { - // JSON method converts undefined to null - const cloned = LiteGraph.cloneObject(obj) - expect(cloned.undefined).toBe(null) - } + + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.undefined).toBe(undefined) + expect("undefined" in cloned).toBe(true) }) - it("should handle circular references when structuredClone is available", () => { + it("should handle circular references", () => { const obj: any = { name: "test" } obj.self = obj - - if (typeof structuredClone !== "undefined") { - const cloned = LiteGraph.cloneObject(obj) - expect(cloned.name).toBe("test") - expect(cloned.self).toBe(cloned) // Circular reference preserved - expect(cloned).not.toBe(obj) // But it's still a clone - } else { - // Without structuredClone, should throw - expect(() => LiteGraph.cloneObject(obj)).toThrow() - } + + const cloned = LiteGraph.cloneObject(obj) + expect(cloned.name).toBe("test") + expect(cloned.self).toBe(cloned) // Circular reference preserved + expect(cloned).not.toBe(obj) // But it's still a clone }) - it("should fallback to JSON method when structuredClone fails", () => { - // Mock structuredClone to throw an error - const originalStructuredClone = globalThis.structuredClone - const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) - - try { - // @ts-expect-error - mocking global - globalThis.structuredClone = () => { - throw new Error("Mock structuredClone failure") - } - - LiteGraph.debug = true - const obj = { a: 1, b: "test" } - const cloned = LiteGraph.cloneObject(obj) - - expect(cloned).toEqual(obj) - expect(cloned).not.toBe(obj) - expect(consoleSpy).toHaveBeenCalledWith( - "structuredClone failed, falling back to JSON method:", - expect.any(Error), - ) - } finally { - globalThis.structuredClone = originalStructuredClone - consoleSpy.mockRestore() - } - }) it("should handle the target parameter correctly", () => { const source = { a: 1, b: 2 }