Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion __tests__/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
immerable,
enablePatches,
enableMapSet,
enableArrayMethods
enableArrayMethods,
applyPatches
} from "../src/immer"

import {
Expand Down Expand Up @@ -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}]}
Expand Down
24 changes: 23 additions & 1 deletion src/core/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,10 @@ export const objectTraps: ProxyHandler<ProxyState> = {
}
// 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
Expand Down Expand Up @@ -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
Expand Down
Loading