From 88e362a42ab88997a2925a9fa7a1d2ff8dd58d71 Mon Sep 17 00:00:00 2001 From: unadlib Date: Fri, 1 May 2026 01:12:57 +0800 Subject: [PATCH 1/7] fix array perf --- package.json | 1 + src/array.ts | 343 ++++++++++++++++ src/current.ts | 10 + src/draft.ts | 83 +++- src/interface.ts | 1 + src/utils/copy.ts | 12 +- src/utils/draft.ts | 46 +++ src/utils/finalize.ts | 24 ++ test/array.test.ts | 523 ++++++++++++++++++++++++- test/benchmark/array-current-vs-npm.ts | 318 +++++++++++++++ 10 files changed, 1347 insertions(+), 14 deletions(-) create mode 100644 src/array.ts create mode 100644 test/benchmark/array-current-vs-npm.ts diff --git a/package.json b/package.json index ac31ab46..0f235aee 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "benchmark:base": "NODE_ENV='production' ts-node test/performance/benchmark.ts", "benchmark:object": "NODE_ENV='production' ts-node test/performance/benchmark-object.ts", "benchmark:array": "NODE_ENV='production' ts-node test/performance/benchmark-array.ts", + "benchmark:array-npm": "yarn build && MUTATIVE_BENCH_USE_DIST=true NODE_ENV='production' ts-node test/benchmark/array-current-vs-npm.ts", "benchmark:class": "NODE_ENV='production' ts-node test/performance/benchmark-class.ts", "performance:read-only": "yarn build && NODE_ENV='production' ts-node test/performance/read-draft/index.ts", "performance:immer": "cd test/__immer_performance_tests__ && NODE_ENV='production' ts-node add-data.ts && NODE_ENV='production' ts-node todo.ts && NODE_ENV='production' ts-node incremental.ts", diff --git a/src/array.ts b/src/array.ts new file mode 100644 index 00000000..b64110e1 --- /dev/null +++ b/src/array.ts @@ -0,0 +1,343 @@ +import { dataTypes } from './constant'; +import { internal } from './internal'; +import { generatePatches } from './patch'; +import { checkReadable } from './unsafe'; +import { + ensureShallowCopy, + getProxyDraft, + isDraftable, + isEqual, + isArrayIndex, + isAssignedArrayIndex, + isLazyArrayDraft, + isOriginalArrayValue, + markChanged, + markFinalization, + setAssignedArrayIndex, +} from './utils'; +import type { ProxyDraft } from './interface'; + +function toInteger(value: any) { + const number = +value; + if (Number.isNaN(number) || number === 0) return 0; + if (!Number.isFinite(number)) return number; + return number < 0 ? Math.ceil(number) : Math.floor(number); +} + +function normalizeStart(value: any, length: number) { + const start = toInteger(value); + if (start === -Infinity) return 0; + if (start < 0) return Math.max(length + start, 0); + return Math.min(start, length); +} + +function isSameArrayValue(value: any, originalValue: any) { + const proxyDraft = getProxyDraft(value); + if (proxyDraft) { + return !proxyDraft.operated && isEqual(proxyDraft.original, originalValue); + } + return isEqual(value, originalValue); +} + +function markArrayChanged(target: ProxyDraft) { + if (target.operated) return; + const copy = target.copy!; + const original = target.original; + if (copy.length !== original.length) { + markChanged(target); + return; + } + for (let index = 0; index < copy.length; index += 1) { + const copyHasValue = index in copy; + const originalHasValue = index in original; + if ( + copyHasValue !== originalHasValue || + (copyHasValue && !isSameArrayValue(copy[index], original[index])) + ) { + markChanged(target); + return; + } + } +} + +function getLazyArrayTarget(value: any) { + const target = getProxyDraft(value); + return target && isLazyArrayDraft(target) + ? (target as ProxyDraft) + : null; +} + +function applyNativeArrayMethod( + method: 'shift' | 'unshift' | 'splice' | 'reverse', + value: any, + args: any[] +) { + return Reflect.apply((Array.prototype as any)[method], value, args); +} + +function copyArrayValues(source: any[], target: any[]) { + for (let index = 0; index < source.length; index += 1) { + if (index in source) { + target[index] = source[index]; + } + } + target.length = source.length; +} + +function applyCachedArrayDrafts(target: ProxyDraft, copy: any[]) { + const original = target.original; + if (target.arrayDrafts?.size) { + target.arrayDrafts.forEach((draft, index) => { + if (index in copy && copy[index] === original[index]) { + copy[index] = draft; + } + }); + } +} + +function getAssignedFlags(target: ProxyDraft, length: number) { + return Array.from({ length }, (_, index) => + isAssignedArrayIndex(target, index) + ); +} + +function setAssignedFlags(target: ProxyDraft, flags: boolean[]) { + target.assignedMap?.forEach((_, key) => { + if (isArrayIndex(key)) { + target.assignedMap!.delete(key); + } + }); + flags.forEach((assigned, index) => { + setAssignedArrayIndex(target, index, assigned); + }); +} + +function isAssignedInsertedValue( + target: ProxyDraft, + key: number, + value: any +) { + const original = target.original; + return !(key in original) || !isEqual(value, original[key]); +} + +function createArrayCopy(target: ProxyDraft) { + const original = target.original; + const pendingCopy = new Array(original.length); + copyArrayValues(original, pendingCopy); + applyCachedArrayDrafts(target, pendingCopy); + // Species constructors can re-enter the draft while the copy is being built. + target.copy = pendingCopy as any; + + const copy = arraySpeciesCreate(original, 0); + copyArrayValues(target.copy as any[], copy); + return copy; +} + +function prepareArrayCopy(target: ProxyDraft) { + if (!target.copy) { + target.copy = createArrayCopy(target) as any; + } + return target.copy!; +} + +function draftArrayValue( + target: ProxyDraft, + key: number, + value: any, + options: { cache?: boolean; setCopy?: boolean } = {} +) { + const valueProxyDraft = getProxyDraft(value); + if (valueProxyDraft) return value; + let markResult: any; + if (target.options.mark) { + markResult = target.options.mark(value, dataTypes); + if (markResult === dataTypes.mutable) { + if (target.options.strict) { + checkReadable(value, target.options, true); + } + return value; + } + } + if (target.options.strict) { + checkReadable(value, target.options); + } + if ( + target.finalized || + !isDraftable(value, target.options) || + isAssignedArrayIndex(target, key) || + !isOriginalArrayValue(target, value) + ) { + return value; + } + if (options.cache && !target.copy) { + const cachedDraft = target.arrayDrafts?.get(key); + if (cachedDraft) return cachedDraft; + } + const draft = internal.createDraft({ + original: value, + parentDraft: target, + key, + finalities: target.finalities, + options: target.options, + }); + if (options.cache && !target.copy) { + target.arrayDrafts = target.arrayDrafts ?? new Map(); + target.arrayDrafts.set(key, draft); + } else if (options.setCopy && target.copy?.[key] === value) { + target.copy![key] = draft; + } + if (typeof markResult === 'function') { + const proxyDraft = getProxyDraft(draft)!; + ensureShallowCopy(proxyDraft); + markChanged(proxyDraft); + return proxyDraft.copy; + } + return draft; +} + +function draftRemovedValues( + target: ProxyDraft, + start: number, + speciesSource: any[], + source: any[], + count: number +) { + const result = arraySpeciesCreate(speciesSource, count); + for (let index = 0; index < count; index += 1) { + const key = start + index; + if (key in source) { + result[index] = draftArrayValue(target, key, source[key]); + } + } + result.length = count; + return result; +} + +function arraySpeciesCreate(original: any, length: number) { + if (!Array.isArray(original)) { + return new Array(length); + } + let constructor: any = original.constructor; + if (constructor === undefined) { + return new Array(length); + } + if ( + constructor === null || + (typeof constructor !== 'object' && typeof constructor !== 'function') + ) { + throw new TypeError(`Array species constructor is not a constructor`); + } + constructor = constructor[Symbol.species]; + if (constructor == null) { + return new Array(length); + } + return new constructor(length); +} + +function markInsertedValues( + target: ProxyDraft, + start: number, + count: number +) { + for (let index = 0; index < count; index += 1) { + const key = start + index; + markFinalization(target, key, target.copy![key], generatePatches); + } +} + +export const arrayHandler = { + shift() { + const target = getLazyArrayTarget(this); + if (!target) return applyNativeArrayMethod('shift', this, []); + const source = target.copy ?? target.original; + if (source.length === 0) return undefined; + const copy = prepareArrayCopy(target); + const assignedFlags = getAssignedFlags(target, copy.length); + const result = draftArrayValue(target, 0, copy[0]); + Array.prototype.shift.call(copy); + Array.prototype.shift.call(assignedFlags); + setAssignedFlags(target, assignedFlags); + markArrayChanged(target); + return result; + }, + unshift(...values: any[]) { + const target = getLazyArrayTarget(this); + if (!target) return applyNativeArrayMethod('unshift', this, values); + if (values.length === 0) return (target.copy ?? target.original).length; + const copy = prepareArrayCopy(target); + const assignedFlags = getAssignedFlags(target, copy.length); + const insertedFlags = values.map((value, index) => + isAssignedInsertedValue(target, index, value) + ); + const result = Array.prototype.unshift.apply(copy, values); + Array.prototype.unshift.apply(assignedFlags, insertedFlags); + setAssignedFlags(target, assignedFlags); + markInsertedValues(target, 0, values.length); + markArrayChanged(target); + return result; + }, + splice(...args: any[]) { + const target = getLazyArrayTarget(this); + if (!target) return applyNativeArrayMethod('splice', this, args); + if (args.length === 0) { + const result = arraySpeciesCreate(target.copy ?? target.original, 0); + result.length = 0; + return result; + } + const source = target.copy ?? target.original; + const length = source.length; + const start = normalizeStart(args[0], length); + const deleteCount = + args.length === 1 + ? length - start + : Math.min(Math.max(toInteger(args[1]), 0), length - start); + const insertedCount = Math.max(args.length - 2, 0); + const copy = prepareArrayCopy(target); + if (copy.length !== length) { + copy.length = length; + } + const assignedFlags = getAssignedFlags(target, length); + assignedFlags.length = length; + const insertedFlags = args.slice(2).map((value, index) => + isAssignedInsertedValue(target, start + index, value) + ); + const result = draftRemovedValues(target, start, source, copy, deleteCount); + if (args.length === 1) { + Reflect.apply(Array.prototype.splice, copy, [start]); + Reflect.apply(Array.prototype.splice, assignedFlags, [start]); + } else { + Reflect.apply(Array.prototype.splice, copy, [ + start, + deleteCount, + ...args.slice(2), + ]); + Reflect.apply(Array.prototype.splice, assignedFlags, [ + start, + deleteCount, + ...insertedFlags, + ]); + } + setAssignedFlags(target, assignedFlags); + markInsertedValues(target, start, insertedCount); + markArrayChanged(target); + return result; + }, + reverse() { + const target = getLazyArrayTarget(this); + if (!target) return applyNativeArrayMethod('reverse', this, []); + const source = target.copy ?? target.original; + if (source.length <= 1) return this; + const copy = prepareArrayCopy(target); + const assignedFlags = getAssignedFlags(target, copy.length); + Array.prototype.reverse.call(copy); + Array.prototype.reverse.call(assignedFlags); + setAssignedFlags(target, assignedFlags); + markArrayChanged(target); + return this; + }, +}; + +export const arrayHandlerKeys = Reflect.ownKeys(arrayHandler); + +export { draftArrayValue }; diff --git a/src/current.ts b/src/current.ts index bfc451d5..986d928c 100644 --- a/src/current.ts +++ b/src/current.ts @@ -9,6 +9,7 @@ import { isDraft, isDraftable, isEqual, + isLazyArrayDraft, set, shallowCopy, } from './utils'; @@ -68,6 +69,15 @@ function getCurrent(target: any) { if (proxyDraft && !proxyDraft.operated) return proxyDraft.original; let currentValue: any; function ensureShallowCopy() { + if (proxyDraft && isLazyArrayDraft(proxyDraft) && !proxyDraft.copy) { + currentValue = shallowCopy(proxyDraft.original, proxyDraft.options); + proxyDraft.arrayDrafts?.forEach((draft, index) => { + if (index in currentValue) { + currentValue[index] = draft; + } + }); + return; + } currentValue = type === DraftType.Map ? !isBaseMapInstance(target) diff --git a/src/draft.ts b/src/draft.ts index d52d6bf4..893a4f44 100644 --- a/src/draft.ts +++ b/src/draft.ts @@ -7,6 +7,7 @@ import { Operation, } from './interface'; import { dataTypes, PROXY_DRAFT } from './constant'; +import { arrayHandler, arrayHandlerKeys, draftArrayValue } from './array'; import { mapHandler, mapHandlerKeys } from './map'; import { setHandler, setHandlerKeys } from './set'; import { internal } from './internal'; @@ -27,9 +28,14 @@ import { set, revokeProxy, finalizeSetValue, + finalizeArrayValue, markFinalization, finalizePatches, isDraft, + getArrayIndex, + isArrayIndex, + isLazyArrayDraft, + isOriginalArrayValue, } from './utils'; import { checkReadable } from './unsafe'; import { generatePatches } from './patch'; @@ -82,6 +88,37 @@ const proxyHandler: ProxyHandler = { } if (!has(source, key)) { + if (isLazyArrayDraft(target) && arrayHandlerKeys.includes(key as any)) { + if (has(target.original as any, key)) { + return (target.original as any)[key]; + } + const sourceDesc = getDescriptor(source, key); + if (sourceDesc) { + if ( + 'value' in sourceDesc && + sourceDesc.value === (Array.prototype as any)[key] + ) { + const handle = arrayHandler[ + key as keyof typeof arrayHandler + ] as Function; + return handle; + } + return 'value' in sourceDesc + ? sourceDesc.value + : sourceDesc.get?.call(target.proxy); + } + const originalDesc = getDescriptor(target.original as any, key); + if ( + originalDesc && + 'value' in originalDesc && + originalDesc.value === (Array.prototype as any)[key] + ) { + const handle = arrayHandler[ + key as keyof typeof arrayHandler + ] as Function; + return handle; + } + } const desc = getDescriptor(source, key); return desc ? `value` in desc @@ -97,6 +134,19 @@ const proxyHandler: ProxyHandler = { if (target.finalized || !isDraftable(value, target.options)) { return value; } + if ( + isLazyArrayDraft(target) && + isArrayIndex(key) && + isOriginalArrayValue(target, value) + ) { + const index = getArrayIndex(key); + if (!target.copy && value === peek(target.original, key)) { + return draftArrayValue(target, index, value, { cache: true }); + } + if (target.copy) { + return draftArrayValue(target, index, value, { setCopy: true }); + } + } // Ensure that the assigned values are not drafted if (value === peek(target.original, key)) { ensureShallowCopy(target); @@ -128,15 +178,10 @@ const proxyHandler: ProxyHandler = { `Map/Set draft does not support any property assignment.` ); } - let _key: number; if ( target.type === DraftType.Array && key !== 'length' && - !( - Number.isInteger((_key = Number(key))) && - _key >= 0 && - (key === 0 || _key === 0 || String(_key) === String(key)) - ) + !isArrayIndex(key) ) { throw new Error( `Only supports setting array indices and the 'length' property.` @@ -148,10 +193,15 @@ const proxyHandler: ProxyHandler = { desc.set.call(target.proxy, value); return true; } - const current = peek(latest(target), key); + const current = + isLazyArrayDraft(target) && isArrayIndex(key) && !target.copy + ? (target.arrayDrafts?.get(getArrayIndex(key)) ?? + peek(latest(target), key)) + : peek(latest(target), key); const currentProxyDraft = getProxyDraft(current); if (currentProxyDraft && isEqual(currentProxyDraft.original, value)) { // !case: ignore the case of assigning the original draftable value to a draft + ensureShallowCopy(target); target.copy![key] = value; target.assignedMap = target.assignedMap ?? new Map(); target.assignedMap.set(key, false); @@ -185,11 +235,15 @@ const proxyHandler: ProxyHandler = { const source = latest(target); const descriptor = Reflect.getOwnPropertyDescriptor(source, key); if (!descriptor) return descriptor; + const value = + isLazyArrayDraft(target) && isArrayIndex(key) && !target.copy + ? (target.arrayDrafts?.get(getArrayIndex(key)) ?? source[key]) + : source[key]; return { writable: true, configurable: target.type !== DraftType.Array || key !== 'length', enumerable: descriptor.enumerable, - value: source[key], + value, }; }, getPrototypeOf(target: ProxyDraft) { @@ -249,17 +303,20 @@ export function createDraft(createDraftOptions: { if (key || 'key' in createDraftOptions) { proxyDraft.key = key; } - const { proxy, revoke } = Proxy.revocable( - type === DraftType.Array ? Object.assign([], proxyDraft) : proxyDraft, - proxyHandler - ); + const proxyTarget = + type === DraftType.Array ? Object.assign([], proxyDraft) : proxyDraft; + const { proxy, revoke } = Proxy.revocable(proxyTarget, proxyHandler); finalities.revoke.push(revoke); + proxyTarget.proxy = proxy; proxyDraft.proxy = proxy; if (parentDraft) { const target = parentDraft; target.finalities.draft.push((patches, inversePatches) => { const oldProxyDraft = getProxyDraft(proxy)!; // if target is a Set draft, `setMap` is the real Set copies proxy mapping. + if (target.type === DraftType.Array && !target.copy) { + ensureShallowCopy(target); + } let copy = target.type === DraftType.Set ? target.setMap : target.copy; const draft = get(copy, key!); const proxyDraft = getProxyDraft(draft); @@ -270,6 +327,7 @@ export function createDraft(createDraftOptions: { updatedValue = getValue(draft); } finalizeSetValue(proxyDraft); + finalizeArrayValue(proxyDraft); finalizePatches(proxyDraft, generatePatches, patches, inversePatches); if (__DEV__ && target.options.enableAutoFreeze) { target.options.updatedValues = @@ -289,6 +347,7 @@ export function createDraft(createDraftOptions: { const target = getProxyDraft(proxy)!; target.finalities.draft.push((patches, inversePatches) => { finalizeSetValue(target); + finalizeArrayValue(target); finalizePatches(target, generatePatches, patches, inversePatches); }); } diff --git a/src/interface.ts b/src/interface.ts index b0eaa7e1..ed663132 100644 --- a/src/interface.ts +++ b/src/interface.ts @@ -47,6 +47,7 @@ export interface ProxyDraft { parent?: ProxyDraft | null; key?: string | number | symbol; setMap?: Map; + arrayDrafts?: Map; assignedMap?: Map; callbacks?: ((patches?: Patches, inversePatches?: Patches) => void)[]; } diff --git a/src/utils/copy.ts b/src/utils/copy.ts index 567892e0..817c3226 100644 --- a/src/utils/copy.ts +++ b/src/utils/copy.ts @@ -1,4 +1,4 @@ -import type { Options, ProxyDraft } from '../interface'; +import { DraftType, type Options, type ProxyDraft } from '../interface'; import { dataTypes } from '../constant'; import { getValue, isDraft, isDraftable } from './draft'; import { isBaseMapInstance, isBaseSetInstance } from './proto'; @@ -91,6 +91,16 @@ export function shallowCopy(original: any, options?: Options) { export function ensureShallowCopy(target: ProxyDraft) { if (target.copy) return; target.copy = shallowCopy(target.original, target.options)!; + if (target.type === DraftType.Array && target.arrayDrafts?.size) { + target.arrayDrafts.forEach((draft, index) => { + if ( + index in (target.copy as any[]) && + (target.copy as any[])[index] === (target.original as any[])[index] + ) { + (target.copy as any[])[index] = draft; + } + }); + } } function deepClone(target: T): T; diff --git a/src/utils/draft.ts b/src/utils/draft.ts index be1f6108..36b3b34a 100644 --- a/src/utils/draft.ts +++ b/src/utils/draft.ts @@ -6,6 +6,52 @@ export function latest(proxyDraft: ProxyDraft): T { return proxyDraft.copy ?? proxyDraft.original; } +export function isLazyArrayDraft(proxyDraft: ProxyDraft) { + return ( + proxyDraft.type === DraftType.Array && !proxyDraft.options.enablePatches + ); +} + +export function isArrayIndex(key: PropertyKey): key is string | number { + let index: number; + return ( + key !== 'length' && + typeof key !== 'symbol' && + Number.isInteger((index = Number(key))) && + index >= 0 && + (key === 0 || index === 0 || String(index) === String(key)) + ); +} + +export function getArrayIndex(key: string | number) { + return Number(key); +} + +export function isAssignedArrayIndex(proxyDraft: ProxyDraft, index: number) { + return ( + proxyDraft.assignedMap?.get(index) === true || + proxyDraft.assignedMap?.get(String(index)) === true + ); +} + +export function setAssignedArrayIndex( + proxyDraft: ProxyDraft, + index: number, + assigned: boolean +) { + if (!assigned) { + proxyDraft.assignedMap?.delete(index); + proxyDraft.assignedMap?.delete(String(index)); + return; + } + proxyDraft.assignedMap = proxyDraft.assignedMap ?? new Map(); + proxyDraft.assignedMap.set(index, true); +} + +export function isOriginalArrayValue(proxyDraft: ProxyDraft, value: any) { + return Array.prototype.indexOf.call(proxyDraft.original, value) !== -1; +} + /** * Check if the value is a draft */ diff --git a/src/utils/finalize.ts b/src/utils/finalize.ts index f545c62d..d3023047 100644 --- a/src/utils/finalize.ts +++ b/src/utils/finalize.ts @@ -85,6 +85,30 @@ export function finalizeSetValue(target: ProxyDraft) { } } +export function finalizeArrayValue(target: ProxyDraft) { + if (target.type === DraftType.Array && target.copy) { + for (let index = 0; index < target.copy.length; index += 1) { + if (!(index in target.copy)) continue; + const value = target.copy[index]; + const proxyDraft = getProxyDraft(value); + if (proxyDraft) { + let updatedValue = proxyDraft.original; + if (proxyDraft.operated) { + updatedValue = getValue(value); + } + finalizeSetValue(proxyDraft); + finalizeArrayValue(proxyDraft); + if (__DEV__ && target.options.enableAutoFreeze) { + target.options.updatedValues = + target.options.updatedValues ?? new WeakMap(); + target.options.updatedValues.set(updatedValue, proxyDraft.original); + } + target.copy[index] = updatedValue; + } + } + } +} + export function finalizePatches( target: ProxyDraft, generatePatches: GeneratePatches, diff --git a/test/array.test.ts b/test/array.test.ts index fe807703..2ba3797d 100644 --- a/test/array.test.ts +++ b/test/array.test.ts @@ -1,4 +1,9 @@ -import { create, isDraft } from '../src'; +import { create, current, isDraft } from '../src'; +import { arrayHandlerKeys } from '../src/array'; + +test('only target array mutation methods are lazily intercepted', () => { + expect(arrayHandlerKeys).toEqual(['shift', 'unshift', 'splice', 'reverse']); +}); test('shift', () => { const obj = { @@ -30,6 +35,476 @@ test('splice', () => { expect(obj.a[0] === state.a.slice(-1)[0]).toBe(false); }); +test('splice normalizes start and deleteCount once', () => { + let startCalls = 0; + let deleteCountCalls = 0; + const start = { + valueOf() { + startCalls += 1; + return 1; + }, + }; + const deleteCount = { + valueOf() { + deleteCountCalls += 1; + return 1; + }, + }; + const obj = { + a: [{ i: 0 }, { i: 1 }, { i: 2 }], + }; + + const state = create(obj, (draft) => { + const [item] = draft.a.splice(start as any, deleteCount as any); + expect(isDraft(item)).toBeTruthy(); + }); + + expect(startCalls).toBe(1); + expect(deleteCountCalls).toBe(1); + expect(state).toEqual({ a: [{ i: 0 }, { i: 2 }] }); +}); + +test('splice uses length captured before argument coercion', () => { + const obj = { + a: [{ i: 0 }, { i: 1 }], + }; + let draftArray: typeof obj.a; + const start = { + valueOf() { + draftArray.push({ i: 2 }); + return 0; + }, + }; + + const state = create(obj, (draft) => { + draftArray = draft.a; + const removed = draft.a.splice(start as any, 10); + expect(removed).toHaveLength(2); + expect(removed.map((item) => item.i)).toEqual([0, 1]); + }); + + expect(state).toEqual({ a: [] }); +}); + +test('splice coerces arguments before array species access', () => { + const events: string[] = []; + const obj = { + a: [{ i: 0 }, { i: 1 }], + }; + Object.defineProperty(obj.a, 'constructor', { + configurable: true, + get() { + events.push('constructor'); + return Array; + }, + }); + const start = { + valueOf() { + events.push('start'); + return 0; + }, + }; + + create(obj, (draft) => { + draft.a.splice(start as any, 1); + }); + + expect(events[0]).toBe('start'); +}); + +test('splice rejects BigInt start and deleteCount like native arrays', () => { + expect(() => { + create([{ i: 0 }, { i: 1 }], (draft) => { + draft.splice(BigInt(1) as any, 1); + }); + }).toThrow(TypeError); + expect(() => { + create([{ i: 0 }, { i: 1 }], (draft) => { + draft.splice(0, BigInt(1) as any); + }); + }).toThrow(TypeError); +}); + +test('splice removed array follows array species', () => { + class RemovedItems extends Array {} + const list = [{ i: 0 }, { i: 1 }, { i: 2 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: RemovedItems, + }, + }); + + create({ list }, (draft) => { + const removed = draft.list.splice(1, 1); + expect(removed).toBeInstanceOf(RemovedItems); + expect(isDraft(removed[0])).toBeTruthy(); + expect(removed[0].i).toBe(1); + + const empty = draft.list.splice(); + expect(empty).toBeInstanceOf(RemovedItems); + expect(empty).toHaveLength(0); + }); +}); + +test('splice removed non-array species has length', () => { + class RemovedItems { + [key: number]: any; + length?: number; + } + const list = [{ i: 0 }, { i: 1 }, { i: 2 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: RemovedItems, + }, + }); + + create({ list }, (draft) => { + const removed = draft.list.splice(1, 1) as any; + expect(removed).toBeInstanceOf(RemovedItems); + expect(removed.length).toBe(1); + expect(Object.keys(removed)).toEqual(['0', 'length']); + expect(isDraft(removed[0])).toBeTruthy(); + expect(removed[0].i).toBe(1); + + const empty = draft.list.splice() as any; + expect(Array.isArray(empty)).toBe(true); + expect(empty.length).toBe(0); + expect(Object.keys(empty)).toEqual([]); + }); +}); + +test('splice removed values are read after array species creation', () => { + let draftArray: any[]; + class RemovedItems { + [key: number]: any; + length?: number; + + constructor(length: number) { + draftArray[0] = { i: 99 }; + this.length = length; + } + } + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: RemovedItems, + }, + }); + + const state = create({ list }, (draft) => { + draftArray = draft.list; + const removed = draft.list.splice(0, 1) as any; + expect(removed).toBeInstanceOf(RemovedItems); + expect(removed[0].i).toBe(99); + }); + + expect(list[0].i).toBe(0); + expect(state.list).toBeInstanceOf(RemovedItems); + expect((state.list as any).length).toBe(1); + expect((state.list as any)[0]).toEqual({ i: 1 }); +}); + +test('array own mutation method is not overridden by lazy handler', () => { + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'splice', { + configurable: true, + value(this: any[]) { + this[0].i = 99; + return 'custom splice'; + }, + }); + + const state = create({ list }, (draft) => { + expect((draft.list as any).splice()).toBe('custom splice'); + }); + + expect(state.list).toEqual([{ i: 99 }, { i: 1 }]); +}); + +test('array own mutation method is still used after lazy array copy', () => { + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'splice', { + configurable: true, + value(this: any[]) { + this[0].i = 99; + return 'custom splice'; + }, + }); + + const state = create({ list }, (draft) => { + draft.list.shift(); + expect((draft.list as any).splice()).toBe('custom splice'); + }); + + expect(state.list).toEqual([{ i: 99 }]); +}); + +test('lazy array mutation methods keep native receiver semantics', () => { + const other = [1, 2, 3]; + const state = create({ list: [{ i: 0 }, { i: 1 }] }, (draft) => { + expect(draft.list.splice).toBe(draft.list.splice); + const removed = draft.list.splice.call(other, 1, 1); + expect(removed).toEqual([2]); + expect(other).toEqual([1, 3]); + }); + + expect(state).toEqual({ list: [{ i: 0 }, { i: 1 }] }); +}); + +test('lazy inserted original array value is not drafted', () => { + const obj0 = { i: 0 }; + const obj1 = { i: 1 }; + const base = { list: [obj0, obj1] }; + + const state = create(base, (draft) => { + draft.list.unshift(obj1); + draft.list[0].i = 9; + }); + + expect(base.list[1].i).toBe(9); + expect(state.list[0]).toBe(obj1); +}); + +test('lazy spliced original array value is not drafted', () => { + const obj0 = { i: 0 }; + const obj1 = { i: 1 }; + const base = { list: [obj0, obj1] }; + + const state = create(base, (draft) => { + draft.list.splice(0, 0, obj1); + draft.list[0].i = 9; + }); + + expect(base.list[1].i).toBe(9); + expect(state.list[0]).toBe(obj1); +}); + +test('lazy inserted assignment flags move with array mutations', () => { + const inserted = { i: -1 }; + const base = { list: [{ i: 0 }] }; + + const state = create(base, (draft) => { + draft.list.unshift(inserted); + draft.list.shift(); + draft.list[0].i = 9; + }); + + expect(base.list[0].i).toBe(0); + expect(state.list[0]).not.toBe(base.list[0]); + expect(state.list[0].i).toBe(9); +}); + +test('array subclass prototype mutation method is not overridden by lazy handler', () => { + class Items extends Array<{ i: number }> {} + Object.defineProperty(Items.prototype, 'splice', { + configurable: true, + value(this: Items) { + this[0].i = 99; + return 'custom prototype splice'; + }, + }); + const list = new Items({ i: 0 }, { i: 1 }); + + const state = create({ list }, (draft) => { + expect((draft.list as any).splice()).toBe('custom prototype splice'); + }); + + expect(state.list[0].i).toBe(99); + expect(state.list).toBeInstanceOf(Items); +}); + +test('lazy array mutation method is not added for arrays without it', () => { + const list = [{ i: 0 }, { i: 1 }] as any; + Object.setPrototypeOf(list, null); + + const state = create({ list }, (draft) => { + expect((draft.list as any).splice).toBeUndefined(); + }); + + expect(state.list).toBe(list); +}); + +test('lazy array mutation methods preserve array subclass copies', () => { + class Items extends Array<{ i: number }> {} + const mutations = [ + (draft: { list: Items }) => { + draft.list.shift(); + }, + (draft: { list: Items }) => { + draft.list.unshift({ i: -1 }); + }, + (draft: { list: Items }) => { + draft.list.splice(1, 1); + }, + (draft: { list: Items }) => { + draft.list.reverse(); + }, + ]; + + mutations.forEach((mutate) => { + const list = new Items({ i: 0 }, { i: 1 }); + const state = create({ list }, mutate); + + expect(Array.isArray(state.list)).toBe(true); + expect(state.list).toBeInstanceOf(Items); + }); +}); + +test('lazy array mutation methods respect array subclass species', () => { + class Items extends Array<{ i: number }> { + static get [Symbol.species]() { + return Array; + } + } + const list = new Items({ i: 0 }, { i: 1 }); + + const state = create({ list }, (draft) => { + draft.list.shift(); + }); + + expect(Array.isArray(state.list)).toBe(true); + expect(state.list).not.toBeInstanceOf(Items); +}); + +test('lazy array mutation copy keeps array subclass constructor effects', () => { + class Items extends Array<{ i: number }> { + created?: boolean; + + constructor(...items: { i: number }[]) { + super(...items); + this.created = true; + } + } + const list = new Items({ i: 0 }, { i: 1 }); + + const state = create({ list }, (draft) => { + draft.list.shift(); + }); + + expect(state.list).toBeInstanceOf(Items); + expect(state.list.created).toBe(true); +}); + +test('lazy array mutation copy follows own array species', () => { + class Copy extends Array<{ i: number }> {} + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: Copy, + }, + }); + + const state = create({ list }, (draft) => { + draft.list.shift(); + }); + + expect(Array.isArray(state.list)).toBe(true); + expect(state.list).toBeInstanceOf(Copy); +}); + +test('lazy array mutation uses copy prototype method after copy exists', () => { + class Copy extends Array<{ i: number }> {} + Object.defineProperty(Copy.prototype, 'splice', { + configurable: true, + value(this: Copy) { + this[0].i = 77; + return 'copy splice'; + }, + }); + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: Copy, + }, + }); + + const state = create({ list }, (draft) => { + draft.list.shift(); + expect((draft.list as any).splice()).toBe('copy splice'); + }); + + expect(state.list).toBeInstanceOf(Copy); + expect(state.list[0].i).toBe(77); +}); + +test('lazy splice removed array follows copy species after copy exists', () => { + class RemovedItems extends Array<{ i: number }> {} + class Copy extends Array<{ i: number }> { + static get [Symbol.species](): any { + return RemovedItems; + } + } + const list = [{ i: 0 }, { i: 1 }, { i: 2 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: Copy, + }, + }); + + create({ list }, (draft) => { + draft.list.shift(); + const removed = draft.list.splice(0, 1); + expect(removed).toBeInstanceOf(RemovedItems); + expect(removed[0].i).toBe(1); + }); +}); + +test('lazy array mutation copy follows non-array species', () => { + class Copy { + [key: number]: any; + length: number; + created = true; + + constructor(length: number) { + this.length = length; + } + } + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: Copy, + }, + }); + + const state = create({ list }, (draft) => { + draft.list.shift(); + }) as any; + + expect(Array.isArray(state.list)).toBe(false); + expect(state.list).toBeInstanceOf(Copy); + expect(state.list.created).toBe(true); + expect(state.list.length).toBe(1); + expect(state.list[0]).toEqual({ i: 1 }); +}); + +test('array original value check ignores custom indexOf', () => { + const obj = { + a: [{ i: 0 }, { i: 1 }], + }; + Object.defineProperty(obj.a, 'indexOf', { + configurable: true, + value() { + return -1; + }, + }); + + const state = create(obj, (draft) => { + draft.a.reverse(); + expect(isDraft(draft.a[0])).toBeTruthy(); + draft.a[0].i = 42; + }); + + expect(obj.a[1].i).toBe(1); + expect(state).toEqual({ a: [{ i: 42 }, { i: 0 }] }); +}); + test('shift with mark', () => { class Test { constructor(public i: number) {} @@ -78,6 +553,52 @@ test('splice with mark', () => { expect(obj.a[0] === state.a.slice(-1)[0]).toBe(false); }); +test('reverse drafts moved original item on later access', () => { + const obj = { + a: [{ i: 0 }, { i: 1 }], + }; + const state = create(obj, (draft) => { + draft.a.reverse(); + draft.a[0].i = 42; + }); + expect(obj.a[1].i).toBe(1); + expect(state).toEqual({ a: [{ i: 42 }, { i: 0 }] }); + expect(state.a[0]).not.toBe(obj.a[1]); +}); + +test('copyWithin no-op after lazy read does not mark array changed', () => { + const obj = { + a: [{ i: 0 }, { i: 1 }, { i: 2 }], + }; + const state = create(obj, (draft) => { + expect(isDraft(draft.a[0])).toBeTruthy(); + draft.a.copyWithin(-3, -3); + }); + expect(state).toBe(obj); +}); + +test('current sees lazy array item changes', () => { + create({ a: [{ i: 0 }, { i: 1 }] }, (draft) => { + draft.a[0].i = 42; + expect(current(draft.a)).toEqual([{ i: 42 }, { i: 1 }]); + }); +}); + +test('sort comparator still receives drafts', () => { + const obj = { + a: [{ i: 2 }, { i: 1 }], + }; + const state = create(obj, (draft) => { + draft.a.sort((a, b) => { + expect(isDraft(a)).toBeTruthy(); + a.i += 10; + return a.i - b.i; + }); + }); + expect(obj).toEqual({ a: [{ i: 2 }, { i: 1 }] }); + expect(state.a.some((item) => item.i > 10)).toBeTruthy(); +}); + // test('shift with custom copy', () => { // const obj = { // a: Array.from({ length: 20 }, (_, i) => new Date(i)), diff --git a/test/benchmark/array-current-vs-npm.ts b/test/benchmark/array-current-vs-npm.ts new file mode 100644 index 00000000..15f10514 --- /dev/null +++ b/test/benchmark/array-current-vs-npm.ts @@ -0,0 +1,318 @@ +/// + +/* eslint-disable import/no-dynamic-require */ +/* eslint-disable @typescript-eslint/no-var-requires */ +/* eslint-disable prefer-template */ +// @ts-nocheck +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import https from 'https'; +import { execFileSync } from 'child_process'; +import { Suite } from 'benchmark'; +import QuickChart from 'quickchart-js'; + +(global as any).__DEV__ = false; +process.env.NODE_ENV = process.env.NODE_ENV || 'production'; + +const currentPackagePath = path.resolve(__dirname, '../../dist/index.js'); +const useCurrentDist = process.env.MUTATIVE_BENCH_USE_DIST !== 'false'; +if (useCurrentDist && !fs.existsSync(currentPackagePath)) { + throw new Error( + `Cannot find current build at ${currentPackagePath}. Run yarn build first, or set MUTATIVE_BENCH_USE_DIST=false to compare workspace source.` + ); +} +const currentPackage = useCurrentDist + ? require(currentPackagePath) + : require('../../src'); +const currentVersion = require('../../package.json').version; +const currentCreate = currentPackage.create; + +const npmSpec = process.env.MUTATIVE_NPM_SPEC || 'mutative@latest'; + +function installNpmMutative(spec: string) { + const tempDir = fs.mkdtempSync( + path.join(os.tmpdir(), 'mutative-npm-benchmark-') + ); + execFileSync( + 'npm', + ['install', '--prefix', tempDir, '--no-save', '--silent', spec], + { + stdio: 'inherit', + } + ); + const packageDir = path.join(tempDir, 'node_modules', 'mutative'); + const packageJson = require(path.join(packageDir, 'package.json')); + const module = require(packageDir); + process.on('exit', () => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + return { + create: module.create, + version: packageJson.version, + }; +} + +const npmMutative = installNpmMutative(npmSpec); + +const config: Parameters[0] = { + type: 'horizontalBar', + data: { + labels: [], + datasets: [ + { + label: `Current ${useCurrentDist ? 'dist' : 'workspace'} v${currentVersion}`, + backgroundColor: 'rgba(54, 162, 235, 0.5)', + borderColor: 'rgb(54, 162, 235)', + borderWidth: 1, + data: [], + }, + { + label: `npm ${npmSpec} resolved v${npmMutative.version}`, + backgroundColor: 'rgba(255, 99, 132, 0.5)', + borderColor: 'rgb(255, 99, 132)', + borderWidth: 1, + data: [], + }, + { + label: 'Array spread reducer', + backgroundColor: 'rgba(75, 192, 123, 0.5)', + borderColor: 'rgb(75, 192, 123)', + borderWidth: 1, + data: [], + }, + ], + }, + options: { + title: { + display: true, + text: 'Current Mutative vs npm Mutative vs Array spread reducer - Array operations', + }, + scales: { + xAxes: [ + { + display: true, + scaleLabel: { + display: true, + labelString: 'Measure(ms/op), lower is better', + }, + }, + ], + yAxes: [ + { + type: 'category', + position: 'left', + display: true, + scaleLabel: { + display: true, + labelString: 'Array operation and size', + }, + }, + ], + }, + }, +}; + +const getData = (size: number) => + Array(size) + .fill(1) + .map((_, key) => ({ value: key })); + +const scenarios = [ + { + name: 'shift remove first item', + mutate(draft: { value: number }[]) { + draft.shift(); + }, + reduce(state: { value: number }[]) { + return [...state.slice(1)]; + }, + }, + { + name: 'unshift insert first item', + mutate(draft: { value: number }[], value: number) { + draft.unshift({ value }); + }, + reduce(state: { value: number }[], value: number) { + return [{ value }, ...state]; + }, + }, + { + name: 'splice move first item', + mutate(draft: { value: number }[], value: number) { + const [item] = draft.splice(0, 1); + item.value = value; + draft.splice(draft.length, 0, item); + }, + reduce(state: { value: number }[], value: number) { + const [item, ...rest] = state; + return [...rest, { ...item, value }]; + }, + }, + { + name: 'reverse then update first', + mutate(draft: { value: number }[], value: number) { + draft.reverse(); + draft[0].value = value; + }, + reduce(state: { value: number }[], value: number) { + const reversed = [...state].reverse(); + return [{ ...reversed[0], value }, ...reversed.slice(1)]; + }, + }, +]; + +const sizes = (process.env.MUTATIVE_BENCH_SIZES || '1000,10000,50000') + .split(',') + .map((value) => Number(value.trim())) + .filter((value) => Number.isFinite(value) && value > 1); + +const benchmarkOptions = { + minSamples: Number(process.env.MUTATIVE_BENCH_MIN_SAMPLES || 20), + minTime: Number(process.env.MUTATIVE_BENCH_MIN_TIME || 0.05), +}; + +function assertSameResult( + scenario: (typeof scenarios)[number], + size: number, + value: number +) { + const baseState = getData(size); + const currentState = currentCreate(baseState, (draft: any) => { + scenario.mutate(draft, value); + }); + const npmState = npmMutative.create(baseState, (draft: any) => { + scenario.mutate(draft, value); + }); + const reducerState = scenario.reduce(baseState, value); + const currentString = JSON.stringify(currentState); + if ( + currentString !== JSON.stringify(npmState) || + currentString !== JSON.stringify(reducerState) + ) { + throw new Error( + `Current, npm, and array spread reducer results are different: ${scenario.name}, size ${size}` + ); + } +} + +function pushMeasure(name: string, ms: number) { + const data = config.data.datasets.find((item) => item.label === name); + data.data.push(ms); +} + +function run(scenario: (typeof scenarios)[number], size: number) { + const label = `${scenario.name} (${size})`; + config.data.labels.push(label); + assertSameResult(scenario, Math.min(size, 100), 42); + + const suite = new Suite(); + let value: number; + let baseState: { value: number }[]; + + const currentLabel = config.data.datasets[0].label; + const npmLabel = config.data.datasets[1].label; + const reducerLabel = config.data.datasets[2].label; + + suite + .add( + currentLabel, + () => { + currentCreate(baseState, (draft: any) => { + scenario.mutate(draft, value); + }); + }, + { + ...benchmarkOptions, + onStart: () => { + value = Math.random(); + baseState = getData(size); + }, + } + ) + .add( + npmLabel, + () => { + npmMutative.create(baseState, (draft: any) => { + scenario.mutate(draft, value); + }); + }, + { + ...benchmarkOptions, + onStart: () => { + value = Math.random(); + baseState = getData(size); + }, + } + ) + .add( + reducerLabel, + () => { + scenario.reduce(baseState, value); + }, + { + ...benchmarkOptions, + onStart: () => { + value = Math.random(); + baseState = getData(size); + }, + } + ) + .on('cycle', (event) => { + const ms = 1000 / event.target.hz; + pushMeasure(event.target.name, ms); + console.log(`${label} - ${event.target.name}: ${ms.toFixed(4)} ms/op`); + }) + .on('complete', function () { + console.log( + `${label}: The fastest method is ${this.filter('fastest').map('name')}` + ); + }) + .run({ async: false }); +} + +for (const scenario of scenarios) { + sizes.forEach((size) => run(scenario, size)); +} + +if (process.env.MUTATIVE_BENCH_WRITE_RESULTS !== 'false') { + const name = path.basename(__filename).replace('.ts', ''); + const currentData = config.data.datasets[0].data; + const npmData = config.data.datasets[1].data; + const reducerData = config.data.datasets[2].data; + const avg = + currentData.reduce((current, value, index) => { + return current + npmData[index] / value; + }, 0) / currentData.length; + const arraySpreadReducerAvg = + currentData.reduce((current, value, index) => { + return current + reducerData[index] / value; + }, 0) / currentData.length; + + const resultsDir = path.resolve(__dirname, './results'); + fs.mkdirSync(resultsDir, { recursive: true }); + const resultPath = path.resolve(resultsDir, 'result.json'); + if (!fs.existsSync(resultPath)) { + fs.writeFileSync(resultPath, '{}'); + } + const data = JSON.parse(fs.readFileSync(resultPath, 'utf8')); + data[name] = { + name, + avg, + arraySpreadReducerAvg, + current: `${useCurrentDist ? 'dist' : 'workspace'}@${currentVersion}`, + npm: `${npmSpec} resolved v${npmMutative.version}`, + arraySpreadReducer: 'Array spread reducer', + }; + fs.writeFileSync(resultPath, JSON.stringify(data, null, 2)); + + const chart = new QuickChart(); + chart.setConfig(config); + const file = fs.createWriteStream(path.resolve(resultsDir, `${name}.jpg`)); + https.get(chart.getUrl(), (response) => { + response.pipe(file); + file.on('finish', () => { + file.close(); + }); + }); +} From 4fac06c53ec21fbec56f5af5465a521c0a7d576f Mon Sep 17 00:00:00 2001 From: unadlib Date: Fri, 1 May 2026 01:23:48 +0800 Subject: [PATCH 2/7] fix lazy array edge cases --- src/array.ts | 22 ++++++++-- src/draft.ts | 24 ++--------- test/array.test.ts | 101 +++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 115 insertions(+), 32 deletions(-) diff --git a/src/array.ts b/src/array.ts index b64110e1..7db0b2ee 100644 --- a/src/array.ts +++ b/src/array.ts @@ -127,11 +127,25 @@ function createArrayCopy(target: ProxyDraft) { copyArrayValues(original, pendingCopy); applyCachedArrayDrafts(target, pendingCopy); // Species constructors can re-enter the draft while the copy is being built. + const previousCopy = target.copy; + const assignedMapSize = target.assignedMap?.size ?? 0; + const draftFinalityCount = target.finalities.draft.length; target.copy = pendingCopy as any; - const copy = arraySpeciesCreate(original, 0); - copyArrayValues(target.copy as any[], copy); - return copy; + try { + const copy = arraySpeciesCreate(original, 0); + copyArrayValues(target.copy as any[], copy); + return copy; + } catch (error) { + if ( + target.copy === pendingCopy && + (target.assignedMap?.size ?? 0) === assignedMapSize && + target.finalities.draft.length === draftFinalityCount + ) { + target.copy = previousCopy; + } + throw error; + } } function prepareArrayCopy(target: ProxyDraft) { @@ -297,12 +311,12 @@ export const arrayHandler = { if (copy.length !== length) { copy.length = length; } + const result = draftRemovedValues(target, start, source, copy, deleteCount); const assignedFlags = getAssignedFlags(target, length); assignedFlags.length = length; const insertedFlags = args.slice(2).map((value, index) => isAssignedInsertedValue(target, start + index, value) ); - const result = draftRemovedValues(target, start, source, copy, deleteCount); if (args.length === 1) { Reflect.apply(Array.prototype.splice, copy, [start]); Reflect.apply(Array.prototype.splice, assignedFlags, [start]); diff --git a/src/draft.ts b/src/draft.ts index 893a4f44..f95ed200 100644 --- a/src/draft.ts +++ b/src/draft.ts @@ -89,29 +89,11 @@ const proxyHandler: ProxyHandler = { if (!has(source, key)) { if (isLazyArrayDraft(target) && arrayHandlerKeys.includes(key as any)) { - if (has(target.original as any, key)) { - return (target.original as any)[key]; - } const sourceDesc = getDescriptor(source, key); - if (sourceDesc) { - if ( - 'value' in sourceDesc && - sourceDesc.value === (Array.prototype as any)[key] - ) { - const handle = arrayHandler[ - key as keyof typeof arrayHandler - ] as Function; - return handle; - } - return 'value' in sourceDesc - ? sourceDesc.value - : sourceDesc.get?.call(target.proxy); - } - const originalDesc = getDescriptor(target.original as any, key); if ( - originalDesc && - 'value' in originalDesc && - originalDesc.value === (Array.prototype as any)[key] + sourceDesc && + 'value' in sourceDesc && + sourceDesc.value === (Array.prototype as any)[key] ) { const handle = arrayHandler[ key as keyof typeof arrayHandler diff --git a/test/array.test.ts b/test/array.test.ts index 2ba3797d..457e1d71 100644 --- a/test/array.test.ts +++ b/test/array.test.ts @@ -161,6 +161,11 @@ test('splice removed non-array species has length', () => { }); create({ list }, (draft) => { + const empty = draft.list.splice() as any; + expect(empty).toBeInstanceOf(RemovedItems); + expect(empty.length).toBe(0); + expect(Object.keys(empty)).toEqual(['length']); + const removed = draft.list.splice(1, 1) as any; expect(removed).toBeInstanceOf(RemovedItems); expect(removed.length).toBe(1); @@ -168,10 +173,7 @@ test('splice removed non-array species has length', () => { expect(isDraft(removed[0])).toBeTruthy(); expect(removed[0].i).toBe(1); - const empty = draft.list.splice() as any; - expect(Array.isArray(empty)).toBe(true); - expect(empty.length).toBe(0); - expect(Object.keys(empty)).toEqual([]); + expect((draft.list as any).splice).toBeUndefined(); }); }); @@ -224,7 +226,7 @@ test('array own mutation method is not overridden by lazy handler', () => { expect(state.list).toEqual([{ i: 99 }, { i: 1 }]); }); -test('array own mutation method is still used after lazy array copy', () => { +test('array own mutation method is not reused after lazy array copy', () => { const list = [{ i: 0 }, { i: 1 }] as any; Object.defineProperty(list, 'splice', { configurable: true, @@ -236,10 +238,10 @@ test('array own mutation method is still used after lazy array copy', () => { const state = create({ list }, (draft) => { draft.list.shift(); - expect((draft.list as any).splice()).toBe('custom splice'); + expect((draft.list as any).splice()).toEqual([]); }); - expect(state.list).toEqual([{ i: 99 }]); + expect(state.list).toEqual([{ i: 1 }]); }); test('lazy array mutation methods keep native receiver semantics', () => { @@ -455,6 +457,91 @@ test('lazy splice removed array follows copy species after copy exists', () => { }); }); +test('lazy array mutation does not add original methods after non-array copy exists', () => { + class Copy { + [key: number]: any; + length: number; + + constructor(length: number) { + this.length = length; + } + } + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: Copy, + }, + }); + + create({ list }, (draft) => { + draft.list.shift(); + expect((draft.list as any).splice).toBeUndefined(); + }); +}); + +test('lazy splice keeps assigned flags from removed species re-entry', () => { + const obj0 = { i: 0 }; + const obj1 = { i: 1 }; + const obj2 = { i: 2 }; + let draftArray: any[] | undefined; + let removedSpeciesCalls = 0; + class Items extends Array<{ i: number }> { + constructor(length: number) { + super(length); + if (length === 1) { + removedSpeciesCalls += 1; + if (removedSpeciesCalls === 1) { + draftArray![2] = obj0; + } + } + } + } + const list = [obj0, obj1, obj2] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: Items, + }, + }); + + const state = create({ list }, (draft) => { + draftArray = draft.list; + draft.list.splice(0, 1); + expect(isDraft(draft.list[1])).toBe(false); + draft.list[1].i = 9; + }); + + expect(obj0.i).toBe(9); + expect(state.list[1]).toBe(obj0); +}); + +test('failed lazy array copy is not reused after species constructor throws', () => { + class ThrowingCopy extends Array<{ i: number }> { + constructor(length: number) { + super(length); + throw new Error('copy boom'); + } + } + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: ThrowingCopy, + }, + }); + + expect(() => { + create({ list }, (draft) => { + try { + draft.list.shift(); + } catch {} + draft.list[0].i = 9; + }); + }).toThrow('copy boom'); + expect(list[0].i).toBe(0); +}); + test('lazy array mutation copy follows non-array species', () => { class Copy { [key: number]: any; From 48049b8d1e0eba5d4a0264b395cd6ebce6be02e7 Mon Sep 17 00:00:00 2001 From: unadlib Date: Fri, 1 May 2026 01:35:12 +0800 Subject: [PATCH 3/7] fix lazy array copy rollback --- src/array.ts | 6 +++--- test/array.test.ts | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/array.ts b/src/array.ts index 7db0b2ee..92f8d105 100644 --- a/src/array.ts +++ b/src/array.ts @@ -128,8 +128,8 @@ function createArrayCopy(target: ProxyDraft) { applyCachedArrayDrafts(target, pendingCopy); // Species constructors can re-enter the draft while the copy is being built. const previousCopy = target.copy; + const wasOperated = target.operated; const assignedMapSize = target.assignedMap?.size ?? 0; - const draftFinalityCount = target.finalities.draft.length; target.copy = pendingCopy as any; try { @@ -139,8 +139,8 @@ function createArrayCopy(target: ProxyDraft) { } catch (error) { if ( target.copy === pendingCopy && - (target.assignedMap?.size ?? 0) === assignedMapSize && - target.finalities.draft.length === draftFinalityCount + target.operated === wasOperated && + (target.assignedMap?.size ?? 0) === assignedMapSize ) { target.copy = previousCopy; } diff --git a/test/array.test.ts b/test/array.test.ts index 457e1d71..71102a81 100644 --- a/test/array.test.ts +++ b/test/array.test.ts @@ -542,6 +542,37 @@ test('failed lazy array copy is not reused after species constructor throws', () expect(list[0].i).toBe(0); }); +test('failed lazy array copy rolls back after read-only species re-entry', () => { + const list = [{ i: 0 }, { i: 1 }] as any; + let draftArray: typeof list | undefined; + class ThrowingCopy extends Array<{ i: number }> { + constructor(length: number) { + super(length); + if (draftArray) { + expect(isDraft(draftArray[0])).toBe(true); + } + throw new Error('copy boom'); + } + } + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: ThrowingCopy, + }, + }); + + expect(() => { + create({ list }, (draft) => { + draftArray = draft.list; + try { + draft.list.shift(); + } catch {} + draft.list[0].i = 9; + }); + }).toThrow('copy boom'); + expect(list[0].i).toBe(0); +}); + test('lazy array mutation copy follows non-array species', () => { class Copy { [key: number]: any; From befee6155700fe8ba14ce5038e80ca4105b52b84 Mon Sep 17 00:00:00 2001 From: unadlib Date: Fri, 1 May 2026 08:06:52 +0800 Subject: [PATCH 4/7] fix lazy array rollback side effects --- src/array.ts | 3 ++- test/array.test.ts | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/array.ts b/src/array.ts index 92f8d105..8277cf75 100644 --- a/src/array.ts +++ b/src/array.ts @@ -139,7 +139,8 @@ function createArrayCopy(target: ProxyDraft) { } catch (error) { if ( target.copy === pendingCopy && - target.operated === wasOperated && + !wasOperated && + !target.operated && (target.assignedMap?.size ?? 0) === assignedMapSize ) { target.copy = previousCopy; diff --git a/test/array.test.ts b/test/array.test.ts index 71102a81..f098c10f 100644 --- a/test/array.test.ts +++ b/test/array.test.ts @@ -573,6 +573,41 @@ test('failed lazy array copy rolls back after read-only species re-entry', () => expect(list[0].i).toBe(0); }); +test('failed lazy array copy keeps write re-entry after prior operation', () => { + const list = [{ i: 0 }, { i: 1 }] as any; + let draftArray: typeof list | undefined; + let speciesCalls = 0; + class SometimesCopy extends Array<{ i: number }> { + constructor(length: number) { + super(length); + speciesCalls += 1; + if (draftArray && speciesCalls === 1) { + draftArray[1].i = 8; + throw new Error('copy boom'); + } + } + } + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: SometimesCopy, + }, + }); + + const state = create({ list }, (draft) => { + draftArray = draft.list; + draft.list[0].i = 7; + try { + draft.list.shift(); + } catch (error) { + expect((error as Error).message).toBe('copy boom'); + } + }); + + expect(state.list).toEqual([{ i: 7 }, { i: 8 }]); + expect(list).toEqual([{ i: 0 }, { i: 1 }]); +}); + test('lazy array mutation copy follows non-array species', () => { class Copy { [key: number]: any; From e4e4cbb9acf9fb91c9a21e4f609e28574f490204 Mon Sep 17 00:00:00 2001 From: unadlib Date: Fri, 1 May 2026 22:39:15 +0800 Subject: [PATCH 5/7] fix lazy array assigned value finalization --- src/utils/finalize.ts | 3 +++ test/array.test.ts | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/utils/finalize.ts b/src/utils/finalize.ts index d3023047..97bdc409 100644 --- a/src/utils/finalize.ts +++ b/src/utils/finalize.ts @@ -5,6 +5,7 @@ import { getPath, getProxyDraft, getValue, + isAssignedArrayIndex, isDraft, isDraftable, isEqual, @@ -104,6 +105,8 @@ export function finalizeArrayValue(target: ProxyDraft) { target.options.updatedValues.set(updatedValue, proxyDraft.original); } target.copy[index] = updatedValue; + } else if (isAssignedArrayIndex(target, index)) { + finalizeAssigned(target, index); } } } diff --git a/test/array.test.ts b/test/array.test.ts index f098c10f..c720b901 100644 --- a/test/array.test.ts +++ b/test/array.test.ts @@ -299,6 +299,38 @@ test('lazy inserted assignment flags move with array mutations', () => { expect(state.list[0].i).toBe(9); }); +test('lazy moved assigned values finalize nested drafts', () => { + const mutations = [ + { + mutate(draft: any) { + draft.list.unshift({ nested: draft.holder }); + draft.list.reverse(); + }, + index: 2, + }, + { + mutate(draft: any) { + draft.list.splice(1, 0, { nested: draft.holder }); + draft.list.shift(); + }, + index: 0, + }, + ]; + + mutations.forEach(({ mutate, index }) => { + const base = { + list: [{ i: 0 }, { i: 1 }], + holder: { j: 1 }, + }; + + const state = create(base, mutate) as any; + + expect(state.list[index].nested.j).toBe(1); + expect(isDraft(state.list[index].nested)).toBe(false); + expect(state.list[index].nested).toBe(base.holder); + }); +}); + test('array subclass prototype mutation method is not overridden by lazy handler', () => { class Items extends Array<{ i: number }> {} Object.defineProperty(Items.prototype, 'splice', { From 1175fa0c5efbb57461f8bc1fe8a771c1f7a62320 Mon Sep 17 00:00:00 2001 From: unadlib Date: Fri, 1 May 2026 22:41:24 +0800 Subject: [PATCH 6/7] fix lazy array copy rollback detection --- src/array.ts | 45 ++++++++++++++++++++++++++++++++++++++++----- test/array.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/array.ts b/src/array.ts index 8277cf75..bc2a866e 100644 --- a/src/array.ts +++ b/src/array.ts @@ -95,6 +95,42 @@ function applyCachedArrayDrafts(target: ProxyDraft, copy: any[]) { } } +function isSamePendingArrayValue(value: any, previousValue: any) { + if (isEqual(value, previousValue)) return true; + const proxyDraft = getProxyDraft(value); + return ( + !!proxyDraft && + !proxyDraft.operated && + isEqual(proxyDraft.original, previousValue) + ); +} + +function hasPendingArrayCopyChanges(copy: any[], previous: any[]) { + if (copy.length !== previous.length) return true; + for (let index = 0; index < copy.length; index += 1) { + const copyHasValue = index in copy; + const previousHasValue = index in previous; + if ( + copyHasValue !== previousHasValue || + (copyHasValue && !isSamePendingArrayValue(copy[index], previous[index])) + ) { + return true; + } + } + return false; +} + +function cachePendingArrayDrafts(target: ProxyDraft, copy: any[]) { + for (let index = 0; index < copy.length; index += 1) { + if (!(index in copy)) continue; + const proxyDraft = getProxyDraft(copy[index]); + if (proxyDraft) { + target.arrayDrafts = target.arrayDrafts ?? new Map(); + target.arrayDrafts.set(index, copy[index]); + } + } +} + function getAssignedFlags(target: ProxyDraft, length: number) { return Array.from({ length }, (_, index) => isAssignedArrayIndex(target, index) @@ -126,10 +162,10 @@ function createArrayCopy(target: ProxyDraft) { const pendingCopy = new Array(original.length); copyArrayValues(original, pendingCopy); applyCachedArrayDrafts(target, pendingCopy); + const previousPendingCopy = new Array(pendingCopy.length); + copyArrayValues(pendingCopy, previousPendingCopy); // Species constructors can re-enter the draft while the copy is being built. const previousCopy = target.copy; - const wasOperated = target.operated; - const assignedMapSize = target.assignedMap?.size ?? 0; target.copy = pendingCopy as any; try { @@ -139,10 +175,9 @@ function createArrayCopy(target: ProxyDraft) { } catch (error) { if ( target.copy === pendingCopy && - !wasOperated && - !target.operated && - (target.assignedMap?.size ?? 0) === assignedMapSize + !hasPendingArrayCopyChanges(pendingCopy, previousPendingCopy) ) { + cachePendingArrayDrafts(target, pendingCopy); target.copy = previousCopy; } throw error; diff --git a/test/array.test.ts b/test/array.test.ts index c720b901..56161291 100644 --- a/test/array.test.ts +++ b/test/array.test.ts @@ -605,6 +605,44 @@ test('failed lazy array copy rolls back after read-only species re-entry', () => expect(list[0].i).toBe(0); }); +test('failed lazy array copy retries after read-only re-entry with prior operation', () => { + const list = [{ i: 0 }, { i: 1 }] as any; + let draftArray: typeof list | undefined; + let speciesCalls = 0; + class SometimesCopy extends Array<{ i: number }> { + constructor(length: number) { + super(length); + speciesCalls += 1; + if (draftArray && speciesCalls === 1) { + expect(isDraft(draftArray[1])).toBe(true); + throw new Error('copy boom'); + } + } + } + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: SometimesCopy, + }, + }); + + const state = create({ list }, (draft) => { + draftArray = draft.list; + draft.list[0].i = 7; + try { + draft.list.shift(); + } catch (error) { + expect((error as Error).message).toBe('copy boom'); + } + }) as any; + + expect(speciesCalls).toBe(2); + expect(state.list).toBeInstanceOf(SometimesCopy); + expect(state.list.length).toBe(2); + expect(Array.from(state.list, (item: any) => item.i)).toEqual([7, 1]); + expect(list).toEqual([{ i: 0 }, { i: 1 }]); +}); + test('failed lazy array copy keeps write re-entry after prior operation', () => { const list = [{ i: 0 }, { i: 1 }] as any; let draftArray: typeof list | undefined; From 815a2305742bfa06574f00b9b035558f3cf31952 Mon Sep 17 00:00:00 2001 From: unadlib Date: Sat, 2 May 2026 00:39:58 +0800 Subject: [PATCH 7/7] fix lazy array species data properties --- src/array.ts | 17 +++++++++++++-- test/array.test.ts | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/array.ts b/src/array.ts index bc2a866e..914d977f 100644 --- a/src/array.ts +++ b/src/array.ts @@ -75,10 +75,19 @@ function applyNativeArrayMethod( return Reflect.apply((Array.prototype as any)[method], value, args); } +function createArrayDataProperty(target: any, index: number, value: any) { + Object.defineProperty(target, index, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +} + function copyArrayValues(source: any[], target: any[]) { for (let index = 0; index < source.length; index += 1) { if (index in source) { - target[index] = source[index]; + createArrayDataProperty(target, index, source[index]); } } target.length = source.length; @@ -257,7 +266,11 @@ function draftRemovedValues( for (let index = 0; index < count; index += 1) { const key = start + index; if (key in source) { - result[index] = draftArrayValue(target, key, source[key]); + createArrayDataProperty( + result, + index, + draftArrayValue(target, key, source[key]) + ); } } result.length = count; diff --git a/test/array.test.ts b/test/array.test.ts index 56161291..04a182c6 100644 --- a/test/array.test.ts +++ b/test/array.test.ts @@ -440,6 +440,32 @@ test('lazy array mutation copy follows own array species', () => { expect(state.list).toBeInstanceOf(Copy); }); +test('lazy array mutation copy creates data properties over species setters', () => { + let setterCalls = 0; + class Copy extends Array<{ i: number }> {} + Object.defineProperty(Copy.prototype, '0', { + configurable: true, + set() { + setterCalls += 1; + }, + }); + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: Copy, + }, + }); + + const state = create({ list }, (draft) => { + draft.list.shift(); + }); + + expect(setterCalls).toBe(0); + expect(Object.prototype.hasOwnProperty.call(state.list, '0')).toBe(true); + expect(state.list[0]).toEqual({ i: 1 }); +}); + test('lazy array mutation uses copy prototype method after copy exists', () => { class Copy extends Array<{ i: number }> {} Object.defineProperty(Copy.prototype, 'splice', { @@ -489,6 +515,32 @@ test('lazy splice removed array follows copy species after copy exists', () => { }); }); +test('lazy splice removed array creates data properties over species setters', () => { + let setterCalls = 0; + class RemovedItems extends Array<{ i: number }> {} + Object.defineProperty(RemovedItems.prototype, '0', { + configurable: true, + set() { + setterCalls += 1; + }, + }); + const list = [{ i: 0 }, { i: 1 }] as any; + Object.defineProperty(list, 'constructor', { + configurable: true, + value: { + [Symbol.species]: RemovedItems, + }, + }); + + create({ list }, (draft) => { + const removed = draft.list.splice(0, 1); + + expect(setterCalls).toBe(0); + expect(Object.prototype.hasOwnProperty.call(removed, '0')).toBe(true); + expect(removed[0].i).toBe(0); + }); +}); + test('lazy array mutation does not add original methods after non-array copy exists', () => { class Copy { [key: number]: any;