diff --git a/src/ImageEditor.tsx b/src/ImageEditor.tsx index a01c9e8..a9e444d 100644 --- a/src/ImageEditor.tsx +++ b/src/ImageEditor.tsx @@ -7,6 +7,7 @@ import React, { } from 'react'; import { loadScript, resetLoader } from './loadScript'; +import { stableKey } from './optionsDiff'; import { ImageEditorInstance, ImageEditorProps, ImageEditorRef } from './types'; function ImageEditorInner( @@ -66,8 +67,8 @@ function ImageEditorInner( // theme/locale/translations apply via updateOptions; everything else in // options requires a remount. const { theme, locale, translations, ...remountOptions } = options; - const remountKey = JSON.stringify(remountOptions); - const updatableKey = JSON.stringify([theme, locale, translations]); + const remountKey = stableKey(remountOptions); + const updatableKey = stableKey([theme, locale, translations]); useEffect(() => { let cancelled = false; @@ -114,7 +115,7 @@ function ImageEditorInner( instance = created; editorRef.current = created; appliedImageRef.current = mountImage; - appliedUpdatableRef.current = JSON.stringify([ + appliedUpdatableRef.current = stableKey([ mountOptions.theme, mountOptions.locale, mountOptions.translations, diff --git a/src/optionsDiff.ts b/src/optionsDiff.ts new file mode 100644 index 0000000..7b8631d --- /dev/null +++ b/src/optionsDiff.ts @@ -0,0 +1,89 @@ +/** + * Stable, comparison-safe serialization for option-diff detection. + * + * `JSON.stringify` is unsuitable as a React-effect key for the `options` + * prop: + * 1. object keys are emitted in insertion order, so semantically identical + * objects that differ only in property declaration order produce + * different strings (a spurious remount that discards unsaved edits, or + * a spurious updateOptions); + * 2. `undefined` and function values are silently dropped, so real changes + * can be invisible to the diff (stale editor configuration); + * 3. a circular reference throws at render time. + * + * `stableKey` returns a canonical string instead: object keys are sorted, + * `undefined` and functions are first-class values (functions keyed by a + * stable reference id), and circular references are emitted as a marker + * rather than recursing forever. Equal structures always serialize to equal + * strings, regardless of key order, without throwing. + */ + +const UNDEFINED = '\uFFFCundefined'; +const CIRCULAR = '\uFFFCcircular'; +const SYMBOL_PREFIX = '\uFFFCsymbol:'; +const FUNCTION_PREFIX = '\uFFFCfunction:'; +const BIGINT_SUFFIX = 'n'; + +// Functions carry no comparable intrinsic value, so key them by reference: +// the same function instance is stable across renders, while a freshly +// created closure is a genuine change the diff must not miss. +const functionIds = new WeakMap(); +let nextFunctionId = 0; + +const functionId = (fn: Function): number => { + const existing = functionIds.get(fn); + if (existing !== undefined) return existing; + const id = nextFunctionId++; + functionIds.set(fn, id); + return id; +}; + +const serialize = (value: unknown, path: Set): string => { + switch (typeof value) { + case 'string': + return JSON.stringify(value); + case 'number': + case 'boolean': + return String(value); + case 'bigint': + return `${value}${BIGINT_SUFFIX}`; + case 'undefined': + return UNDEFINED; + case 'symbol': + return `${SYMBOL_PREFIX}${String(value.description ?? '')}`; + case 'function': + return `${FUNCTION_PREFIX}${functionId(value)}`; + case 'object': + if (value === null) return 'null'; + // A node already on the active path is a cycle: emit a marker and stop + // recursing so a circular `options` object can no longer crash render. + if (path.has(value)) return CIRCULAR; + path.add(value); + let result: string; + if (Array.isArray(value)) { + result = `[${value.map((item) => serialize(item, path)).join(',')}]`; + } else { + const entries = Object.entries(value) + .sort(([left], [right]) => left.localeCompare(right)) + .map( + ([key, entry]) => `${JSON.stringify(key)}:${serialize(entry, path)}` + ) + .join(','); + result = `{${entries}}`; + } + path.delete(value); + return result; + /* v8 ignore next -- every typeof is handled by a case above */ + default: + return 'null'; + } +}; + +/** + * Canonical serializer for structural option diffing. See the module doc for + * the guarantees it provides over `JSON.stringify`. + */ +export const stableKey = (value: unknown): string => { + const path = new Set(); + return serialize(value, path); +}; diff --git a/test/index.test.tsx b/test/index.test.tsx index 5c08aa2..541fb15 100644 --- a/test/index.test.tsx +++ b/test/index.test.tsx @@ -321,6 +321,62 @@ it('remounts the editor when a non-updatable option changes', async () => { expect(mountOptionsOf(1).projectId).toBe(2); }); +it('does not remount or update options when only the key order changes', async () => { + const { rerender } = render( + + ); + await flush(); + + rerender( + + ); + await flush(); + + // Key-order-only differences are semantically identical and must neither + // destroy the editor nor push live options updates. + expect(mockInstance.destroy).not.toHaveBeenCalled(); + expect(createEditor).toHaveBeenCalledTimes(1); + expect(mockInstance.updateOptions).not.toHaveBeenCalled(); +}); + +it('detects a change in a previously-undefined option value', async () => { + const { rerender } = render( + + ); + await flush(); + + rerender( + + ); + await flush(); + + // JSON.stringify would have dropped the undefined name and treated both + // options as identical; the stable diff must see the real change. + expect(mockInstance.destroy).toHaveBeenCalledTimes(1); + expect(createEditor).toHaveBeenCalledTimes(2); + expect(mountOptionsOf(1).user?.name).toBe('Adeel'); +}); + +it('renders without crashing when options contain a circular reference', async () => { + const options: ImageEditorOptions = { projectId: 1 } as ImageEditorOptions; + const self: Record = {}; + self.user = self; + options.user = self as ImageEditorOptions['user']; + + render(); + await flush(); + + // A circular options object must not throw at render; the editor mounts once. + expect(createEditor).toHaveBeenCalledTimes(1); + expect(mockInstance.destroy).not.toHaveBeenCalled(); +}); + it('always invokes the latest callbacks', async () => { const firstOnSave = vi.fn(); const secondOnSave = vi.fn(); diff --git a/test/optionsDiff.test.ts b/test/optionsDiff.test.ts new file mode 100644 index 0000000..7416f81 --- /dev/null +++ b/test/optionsDiff.test.ts @@ -0,0 +1,63 @@ +import { stableKey } from '../src/optionsDiff'; + +describe('stableKey', () => { + it('produces equal keys for objects whose properties are in different order', () => { + expect( + stableKey({ a: { x: 1, y: { p: 2, q: 3 } }, b: 4, c: [1, 2, 3] }) + ).toBe(stableKey({ c: [1, 2, 3], b: 4, a: { y: { q: 3, p: 2 }, x: 1 } })); + }); + + it('distinguishes different values', () => { + expect(stableKey({ a: 1 })).not.toBe(stableKey({ a: 2 })); + expect(stableKey('x')).not.toBe(stableKey('y')); + expect(stableKey([1])).not.toBe(stableKey([1, 2])); + expect(stableKey(null)).not.toBe(stableKey({})); + }); + + it('treats undefined as a first-class value so dropped fields stay visible', () => { + // JSON.stringify omits undefined; here it must be distinguishable from + // both an absent key and a present value. + expect(stableKey({ a: undefined })).not.toBe(stableKey({})); + expect(stableKey({ a: undefined })).not.toBe(stableKey({ a: 1 })); + expect(stableKey({ a: undefined })).toBe(stableKey({ a: undefined })); + expect(stableKey([undefined])).not.toBe(stableKey([])); + }); + + it('keys functions by reference: the same instance is stable, a new one differs', () => { + const callback = () => {}; + expect(stableKey({ cb: callback })).toBe(stableKey({ cb: callback })); + expect(stableKey({ cb: () => {} })).not.toBe(stableKey({ cb: () => {} })); + }); + + it('does not throw on circular references and emits a stable marker', () => { + const first: { name: string; self?: unknown } = { name: 'a' }; + first.self = first; + const second: { name: string; self?: unknown } = { name: 'a' }; + second.self = second; + + expect(() => stableKey(first)).not.toThrow(); + expect(stableKey(first)).toBe(stableKey(second)); + expect(stableKey(first)).not.toBe(stableKey({ name: 'b', self: first })); + + // A node already on the active path inside an array is handled too. + const cycles: unknown[] = [1, 2]; + cycles.push(cycles); + expect(() => stableKey(cycles)).not.toThrow(); + }); + + it('serializes every primitive value type without throwing', () => { + expect(() => + stableKey({ + str: 'text', + num: 42, + bool: false, + big: BigInt(10), + nil: null, + sym: Symbol('sym'), + bareSym: Symbol(), + undef: undefined, + fn: () => {}, + }) + ).not.toThrow(); + }); +});