fix: draft relocated base refs after reverse/sort in array-methods plugin

[spokodev]

Problem

immer's core contract is that the base state passed to produce is never mutated (the docs state the result is produced "without modifying the original baseState"). With enableArrayMethods(), calling reverse() or sort() inside a recipe and then mutating an element breaks this guarantee: the write lands on the user's base object.

Repro

import {produce, isDraft, enableArrayMethods, setAutoFreeze} from "immer"

enableArrayMethods()
setAutoFreeze(false)

const obj3 = {id: 3}
const base = [{id: 1}, {id: 2}, obj3]

const next = produce(base, d => {
  d.reverse()
  // isDraft(d[0]) === false  (bug: should be true)
  d[0].id = 99
})

// base is now [{id:1},{id:2},{id:99}]  and obj3 is {id:99}  -- base mutated

Without the plugin, vanilla immer handles the same recipe correctly.

Root cause

handleReorderingOperation in src/plugins/arrayMethods.ts runs the native reverse/sort directly on state.copy_. Since prepareCopy shallow-copies base_, copy_ still holds the raw base references, and the reorder relocates an un-drafted base object to a new index.

The get trap in src/core/proxy.ts only drafts a child when it sits at its original position (value === peek(state.base_, prop)). After a reorder copy_[0] !== base_[0], so the check falls through and return value hands back the raw base object. Mutating it then mutates the base, and the generated patches/inverse are wrong too.

Fix

Recognize a relocated base reference in the get trap and draft it before exposing it. The extra check only runs after a reorder (allIndicesReassigned_), skips indices the user explicitly assigned, and skips values that are already drafts or non-draftable, so the plugin's lazy-proxy optimization is preserved for the common path.

Tests

Added two cases under "mutating array methods" in __tests__/base.js:

• reverse() then mutate index 0: asserts isDraft(d[0]) is true, base and the relocated object are untouched, and the result is correct.
• sort() then mutate index 0 with produceWithPatches: same base-untouched assertions plus a patch / inverse-patch round-trip.

Both fail on main (in the array-plugin suite) and pass with the fix. Full suite is green (3669 passed, 8 skipped).

[mweststrate]

the indexOf operation here is expensive, do we need it, or should we just return true in any case?

[spokodev]

Good question. It is needed, and it is doing more than a perf guard: indexOf is the only thing that tells a relocated base reference (which we must draft, or a later write reaches the base) apart from any other draftable value the reorder moved into this slot. An assigned value can land here too, and immer's rule is that assigned values are never drafted.

I checked it. With return true, a freshly pushed object gets drafted after a reverse(); with the indexOf membership test it does not:

const fresh = {id: 3}
produce([{id: 1}, {id: 2}], d => {
  d.push(fresh)   // assigned, not a base ref
  d.reverse()     // fresh -> index 0
  isDraft(d[0])   // indexOf: false (correct) | return true: true (drafts an assigned value)
})

The earlier guards (allIndicesReassigned_, !assigned_.get(prop), isDraftable, !value[DRAFT_STATE]) narrow it, but they cannot tell base refs from assigned refs, because after a native reverse/sort the assigned_ indices no longer line up with the new positions. The base-membership test is what keeps us from drafting values immer promised to leave alone. In my tests the final output was the same either way (an untouched draft finalizes back to its source), so this is about the invariant and about not creating proxies for assigned values, not a visible output bug.

On the cost you are right: indexOf is O(n) per hit, so reading a whole reordered array is O(n^2) worst case. If you prefer, I can build a Set of the base references once when allIndicesReassigned_ is set and test membership against that. That makes it O(1) per hit and O(n) overall with the same behavior. I can push that if you want it.

[mweststrate]

Thanks for the catch! Mostly looking great, got one follow up question