From 5380d1cf7bb3b23ad680abd74ec651b934bb6a1f Mon Sep 17 00:00:00 2001 From: Yarchik Date: Thu, 25 Jun 2026 17:14:04 +0100 Subject: [PATCH] fix: draft relocated base refs after reverse/sort in array-methods plugin With enableArrayMethods(), reverse() and sort() run natively on copy_, which still holds raw base references. Reordering relocates an un-drafted base object to an index where it no longer equals base_[prop], so the get trap's positional check misses it and returns the raw base object. Writing to that object mutates the user's base state, violating immer's guarantee that the base state is never modified. Patches and inverse patches are wrong too. Detect a relocated base reference in the get trap (only after a reorder, for indices the user did not explicitly assign) and draft it before exposing it. --- __tests__/base.js | 34 +++++++++++++++++++++++++++++++++- src/core/proxy.ts | 24 +++++++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/__tests__/base.js b/__tests__/base.js index fc249e5a..c293eefa 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -8,7 +8,8 @@ import { immerable, enablePatches, enableMapSet, - enableArrayMethods + enableArrayMethods, + applyPatches } from "../src/immer" import { @@ -356,6 +357,37 @@ function runBaseTest( expect(nextState).toEqual([{value: 1}, {value: 2}, {value: 3}]) }) + it("mutating an element after reverse() does not mutate the base", () => { + const reordered = {id: 3} + const baseState = [{id: 1}, {id: 2}, reordered] + const nextState = produce(baseState, s => { + s.reverse() + // After reverse, the element at index 0 is the relocated base + // object. It must be drafted, otherwise writing to it mutates base. + expect(isDraft(s[0])).toBe(true) + s[0].id = 99 + }) + expect(baseState).toEqual([{id: 1}, {id: 2}, {id: 3}]) + expect(reordered).toEqual({id: 3}) + expect(nextState).toEqual([{id: 99}, {id: 2}, {id: 1}]) + }) + + it("mutating an element after sort() does not mutate the base", () => { + const baseState = [{value: 3}, {value: 1}, {value: 2}] + const [nextState, patches, inverse] = produceWithPatches( + baseState, + s => { + s.sort((a, b) => a.value - b.value) + expect(isDraft(s[0])).toBe(true) + s[0].value = 99 + } + ) + expect(baseState).toEqual([{value: 3}, {value: 1}, {value: 2}]) + expect(nextState).toEqual([{value: 99}, {value: 2}, {value: 3}]) + expect(applyPatches(baseState, patches)).toEqual(nextState) + expect(applyPatches(nextState, inverse)).toEqual(baseState) + }) + describe("push()", () => { test("push single item", () => { const base = {items: [{id: 1}, {id: 2}]} diff --git a/src/core/proxy.ts b/src/core/proxy.ts index a5220b42..27b8d188 100644 --- a/src/core/proxy.ts +++ b/src/core/proxy.ts @@ -146,7 +146,10 @@ export const objectTraps: ProxyHandler = { } // Check for existing draft in modified state. // Assigned values are never drafted. This catches any drafts we created, too. - if (value === peek(state.base_, prop)) { + if ( + value === peek(state.base_, prop) || + isRelocatedBaseRef(state, prop, value) + ) { prepareCopy(state) // Ensure array keys are always numbers const childKey = state.type_ === ArchType.Array ? +(prop as string) : prop @@ -288,6 +291,25 @@ function peek(draft: Drafted, prop: PropertyKey) { return source[prop] } +// Reordering array methods (reverse, sort) from the array-methods plugin run +// natively on `copy_`, which still holds raw base references. This relocates an +// un-drafted base object to a position where it no longer matches `base_[prop]`, +// so the normal positional check above misses it and the get trap would hand back +// the raw base object, breaking the guarantee that the base state is never mutated. +// Detect such a relocated base reference so it is drafted before being exposed. +function isRelocatedBaseRef(state: ImmerState, prop: PropertyKey, value: any) { + if ( + state.type_ !== ArchType.Array || + !(state as ProxyArrayState).allIndicesReassigned_ || + state.assigned_?.get(prop) || + !isDraftable(value) || + value[DRAFT_STATE] + ) { + return false + } + return (state.base_ as AnyArray).indexOf(value) !== -1 +} + function readPropFromProto(state: ImmerState, source: any, prop: PropertyKey) { const desc = getDescriptorFromProto(source, prop) return desc