From 460ddabbae60198f32ff1f2e101b38b61aec2d78 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Mon, 6 Oct 2025 17:09:30 +0200 Subject: [PATCH 1/2] Add many more tests for array behavior --- __tests__/base.js | 1076 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 1062 insertions(+), 14 deletions(-) diff --git a/__tests__/base.js b/__tests__/base.js index e21bb970..d8bdfdc8 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -17,7 +17,7 @@ enablePatches() enableMapSet() vi.setConfig({ - testTimeout: 1000 + testTimeout: 2000 }) const isProd = process.env.NODE_ENV === "production" @@ -263,29 +263,1076 @@ function runBaseTest(name, autoFreeze, useStrictShallowCopy, useListener) { expect(nextState.anArray.length).toBe(baseLength + 1) }) - // Reported here: https://github.com/mweststrate/immer/issues/116 - it("can pop then push", () => { - const nextState = produce([1, 2, 3], s => { - s.pop() - s.push(100) + describe("mutating array methods", () => { + // Reported here: https://github.com/mweststrate/immer/issues/116 + it("can pop then push", () => { + const nextState = produce([1, 2, 3], s => { + s.pop() + s.push(100) + }) + expect(nextState).toEqual([1, 2, 100]) + }) + + it("can be sorted", () => { + const baseState = [{value: 3}, {value: 1}, {value: 2}] + const nextState = produce(baseState, s => { + s.sort((a, b) => a.value - b.value) + }) + expect(nextState).not.toBe(baseState) + expect(nextState).toEqual([{value: 1}, {value: 2}, {value: 3}]) + }) + + it("can be reversed", () => { + const baseState = [{value: 1}, {value: 2}, {value: 3}] + const nextState = produce(baseState, s => { + s.reverse() + }) + expect(nextState).not.toBe(baseState) + expect(nextState).toEqual([{value: 3}, {value: 2}, {value: 1}]) + }) + + it("can be sorted with existing proxies", () => { + const baseState = [{value: 3}, {value: 1}, {value: 2}] + const nextState = produce(baseState, s => { + // First mutate a nested object to create a proxy + s[0].value = 4 + // Then sort the array + s.sort((a, b) => a.value - b.value) + }) + expect(nextState).not.toBe(baseState) + expect(nextState).toEqual([{value: 1}, {value: 2}, {value: 4}]) + }) + + it("can be reversed with existing proxies", () => { + const baseState = [{value: 1}, {value: 2}, {value: 3}] + const nextState = produce(baseState, s => { + // First mutate a nested object to create a proxy + s[1].value = 5 + // Then reverse the array + s.reverse() + }) + expect(nextState).not.toBe(baseState) + expect(nextState).toEqual([{value: 3}, {value: 5}, {value: 1}]) + }) + + it("can be sorted with unmodified existing proxies", () => { + const baseState = [{value: 3}, {value: 1}, {value: 2}] + const nextState = produce(baseState, s => { + // Access a nested object to create a proxy, but don't modify it + const firstValue = s[0].value // This creates a proxy for s[0] + expect(firstValue).toBe(3) // But we don't modify it + + // Then sort the array + s.sort((a, b) => a.value - b.value) + }) + expect(nextState).not.toBe(baseState) + expect(nextState).toEqual([{value: 1}, {value: 2}, {value: 3}]) + }) + + describe("push()", () => { + test("push single item", () => { + const base = {items: [{id: 1}, {id: 2}]} + const result = produce(base, draft => { + draft.items.push({id: 3}) + }) + expect(result.items).toHaveLength(3) + expect(result.items[2].id).toBe(3) + }) + + test("push multiple items", () => { + const base = {items: [{id: 1}]} + const result = produce(base, draft => { + draft.items.push({id: 2}, {id: 3}, {id: 4}) + }) + expect(result.items).toHaveLength(4) + expect(result.items[3].id).toBe(4) + }) + + test("push then mutate pushed item", () => { + const base = {items: [{id: 1}]} + const result = produce(base, draft => { + draft.items.push({id: 2, value: 10}) + draft.items[1].value = 20 + }) + expect(result.items[1].value).toBe(20) + }) + + test("push returns new length", () => { + const base = {items: [1, 2]} + produce(base, draft => { + const newLength = draft.items.push(3, 4) + expect(newLength).toBe(4) + }) + }) + }) + + describe("unshift()", () => { + test("unshift single item", () => { + const base = {items: [{id: 2}, {id: 3}]} + const result = produce(base, draft => { + draft.items.unshift({id: 1}) + }) + expect(result.items).toHaveLength(3) + expect(result.items[0].id).toBe(1) + expect(result.items[1].id).toBe(2) + }) + + test("unshift multiple items", () => { + const base = {items: [{id: 4}]} + const result = produce(base, draft => { + draft.items.unshift({id: 1}, {id: 2}, {id: 3}) + }) + expect(result.items).toHaveLength(4) + expect(result.items[0].id).toBe(1) + expect(result.items[3].id).toBe(4) + }) + + test("unshift then mutate unshifted item", () => { + const base = {items: [{id: 2}]} + const result = produce(base, draft => { + draft.items.unshift({id: 1, value: 10}) + draft.items[0].value = 20 + }) + expect(result.items[0].value).toBe(20) + }) + + test("unshift returns new length", () => { + const base = {items: [3, 4]} + produce(base, draft => { + const newLength = draft.items.unshift(1, 2) + expect(newLength).toBe(4) + }) + }) + }) + + describe("shift()", () => { + test("shift removes first item", () => { + const base = {items: [{id: 1}, {id: 2}, {id: 3}]} + const result = produce(base, draft => { + const removed = draft.items.shift() + expect(removed.id).toBe(1) + }) + expect(result.items).toHaveLength(2) + expect(result.items[0].id).toBe(2) + }) + + test("shift on empty array returns undefined", () => { + const base = {items: []} + produce(base, draft => { + const removed = draft.items.shift() + expect(removed).toBeUndefined() + }) + }) + + test("shift then mutate remaining items", () => { + const base = {items: [{id: 1}, {id: 2, value: 10}]} + const result = produce(base, draft => { + draft.items.shift() + draft.items[0].value = 20 + }) + expect(result.items).toHaveLength(1) + expect(result.items[0].value).toBe(20) + }) + + test("multiple shifts", () => { + const base = {items: [{id: 1}, {id: 2}, {id: 3}]} + const result = produce(base, draft => { + draft.items.shift() + draft.items.shift() + }) + expect(result.items).toHaveLength(1) + expect(result.items[0].id).toBe(3) + }) + }) + + describe("splice() edge cases", () => { + test("splice with only deleteCount (no items to add)", () => { + const base = {items: [{id: 1}, {id: 2}, {id: 3}, {id: 4}]} + const result = produce(base, draft => { + const removed = draft.items.splice(1, 2) + expect(removed).toHaveLength(2) + expect(removed[0].id).toBe(2) + }) + expect(result.items).toHaveLength(2) + expect(result.items[0].id).toBe(1) + expect(result.items[1].id).toBe(4) + }) + + test("splice with negative start index", () => { + const base = {items: [{id: 1}, {id: 2}, {id: 3}]} + const result = produce(base, draft => { + draft.items.splice(-1, 1, {id: 99}) + }) + expect(result.items[2].id).toBe(99) + }) + + test("splice at start (index 0)", () => { + const base = {items: [{id: 1}, {id: 2}]} + const result = produce(base, draft => { + draft.items.splice(0, 0, {id: 0}) + }) + expect(result.items[0].id).toBe(0) + expect(result.items).toHaveLength(3) + }) + + test("splice at end", () => { + const base = {items: [{id: 1}, {id: 2}]} + const result = produce(base, draft => { + draft.items.splice(2, 0, {id: 3}) + }) + expect(result.items[2].id).toBe(3) + }) + + test("splice then mutate spliced-in items", () => { + const base = {items: [{id: 1}, {id: 3}]} + const result = produce(base, draft => { + draft.items.splice(1, 0, {id: 2, value: 10}) + draft.items[1].value = 20 + }) + expect(result.items[1].value).toBe(20) + }) + }) + + describe("combined operations", () => { + test("sort then push", () => { + const base = {items: [{value: 3}, {value: 1}]} + const result = produce(base, draft => { + draft.items.sort((a, b) => a.value - b.value) + draft.items.push({value: 4}) + }) + expect(result.items.map(i => i.value)).toEqual([1, 3, 4]) + }) + + test("reverse then unshift", () => { + const base = {items: [{id: 1}, {id: 2}, {id: 3}]} + const result = produce(base, draft => { + draft.items.reverse() + draft.items.unshift({id: 0}) + }) + expect(result.items.map(i => i.id)).toEqual([0, 3, 2, 1]) + }) + + test("splice then sort", () => { + const base = {items: [{value: 5}, {value: 2}, {value: 8}]} + const result = produce(base, draft => { + draft.items.splice(1, 1, {value: 1}) + draft.items.sort((a, b) => a.value - b.value) + }) + expect(result.items.map(i => i.value)).toEqual([1, 5, 8]) + }) + + test("push, pop, push sequence", () => { + const base = {items: [{id: 1}]} + const result = produce(base, draft => { + draft.items.push({id: 2}) + draft.items.pop() + draft.items.push({id: 3}) + }) + expect(result.items.map(i => i.id)).toEqual([1, 3]) + }) + }) + + describe("bulk operations with pre-existing proxies", () => { + test("access items before sort", () => { + const base = {items: [{value: 3}, {value: 1}, {value: 2}]} + const result = produce(base, draft => { + // Access all items to create proxies + draft.items.forEach(item => item.value) + // Then sort + draft.items.sort((a, b) => a.value - b.value) + }) + expect(result.items.map(i => i.value)).toEqual([1, 2, 3]) + }) + + test("mutate items before reverse", () => { + const base = { + items: [ + {id: 1, value: 10}, + {id: 2, value: 20} + ] + } + const result = produce(base, draft => { + // Mutate to create modified proxies + draft.items[0].value = 15 + draft.items[1].value = 25 + // Then reverse + draft.items.reverse() + }) + expect(result.items[0].id).toBe(2) + expect(result.items[0].value).toBe(25) + expect(result.items[1].id).toBe(1) + expect(result.items[1].value).toBe(15) + }) + }) + + describe("return values", () => { + test("pop returns removed item", () => { + const base = {items: [{id: 1}, {id: 2}]} + produce(base, draft => { + const removed = draft.items.pop() + expect(removed.id).toBe(2) + // Verify we can mutate the returned item + removed.modified = true + expect(draft.items[0].modified).toBeUndefined() + }) + }) + + test("shift returns removed item", () => { + const base = {items: [{id: 1}, {id: 2}]} + produce(base, draft => { + const removed = draft.items.shift() + expect(removed.id).toBe(1) + }) + }) + + test("splice returns array of removed items", () => { + const base = {items: [{id: 1}, {id: 2}, {id: 3}]} + produce(base, draft => { + const removed = draft.items.splice(0, 2) + expect(removed).toHaveLength(2) + expect(removed[0].id).toBe(1) + expect(removed[1].id).toBe(2) + }) + }) }) - expect(nextState).toEqual([1, 2, 100]) }) - it("can be sorted", () => { - const baseState = [3, 1, 2] - const nextState = produce(baseState, s => { - s.sort() + describe("non-mutating array methods", () => { + // Test data factory + const createTestData = () => ({ + items: [ + {id: 1, value: 10, nested: {count: 1}}, + {id: 2, value: 20, nested: {count: 2}}, + {id: 3, value: 30, nested: {count: 3}}, + {id: 4, value: 40, nested: {count: 4}}, + {id: 5, value: 50, nested: {count: 5}} + ], + other: {data: "test"} + }) + + describe("filter()", () => { + test("returns new array with filtered items", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 25) + expect(filtered).toHaveLength(3) + expect(filtered[0].id).toBe(3) + }) + expect(result).toBe(base) // No modifications + }) + + test("mutations to filtered items are reflected in result", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 25) + // Verify filtered items are drafts + expect(isDraft(filtered[0])).toBe(true) + filtered[0].value = 999 + }) + expect(result.items[2].value).toBe(999) // id: 3 is at index 2 + expect(result.items[0].value).toBe(10) // Unchanged + // Verify base state unchanged + expect(base.items[2].value).toBe(30) + // Verify result is a copy + expect(result.items[2]).not.toBe(base.items[2]) + }) + + test("mutations to nested properties work correctly", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.id > 2) + // Verify filtered items are drafts + expect(isDraft(filtered[0])).toBe(true) + filtered[0].nested.count = 100 + }) + expect(result.items[2].nested.count).toBe(100) + // Verify base state unchanged + expect(base.items[2].nested.count).toBe(3) + // Verify result is a copy + expect(result.items[2].nested).not.toBe(base.items[2].nested) + }) + + test("multiple mutations to different filtered items", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 15) + // Verify all filtered items are drafts + filtered.forEach(item => expect(isDraft(item)).toBe(true)) + filtered[0].value = 200 // id: 2 + filtered[1].value = 300 // id: 3 + filtered[2].value = 400 // id: 4 + }) + expect(result.items[1].value).toBe(200) + expect(result.items[2].value).toBe(300) + expect(result.items[3].value).toBe(400) + // Verify base state unchanged + expect(base.items[1].value).toBe(20) + expect(base.items[2].value).toBe(30) + expect(base.items[3].value).toBe(40) + }) + + test("filtered array can be assigned to another property", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 25) + draft.other.filtered = filtered + }) + expect(result.other.filtered).toHaveLength(3) + expect(result.other.filtered[0].id).toBe(3) + }) + + test("cross-reference: mutating filtered item affects original location", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.id === 3) + draft.other.ref = filtered[0] + filtered[0].value = 999 + }) + expect(result.items[2].value).toBe(999) + expect(result.other.ref.value).toBe(999) + }) + + test("read-only access doesn't cause mutations", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 25) + const sum = filtered.reduce((acc, item) => acc + item.value, 0) + expect(sum).toBe(120) // 30 + 40 + 50 + }) + expect(result).toBe(base) // No modifications + }) + }) + + describe("map()", () => { + test("returns new array with mapped items", () => { + const base = createTestData() + const result = produce(base, draft => { + const mapped = draft.items.map(item => item.nested) + expect(mapped).toHaveLength(5) + expect(mapped[0].count).toBe(1) + }) + expect(result).toBe(base) + }) + + test("mutations to mapped nested objects work", () => { + const base = createTestData() + const result = produce(base, draft => { + const nested = draft.items.map(item => item.nested) + // Verify mapped items are drafts + expect(isDraft(nested[0])).toBe(true) + nested[0].count = 100 + }) + expect(result.items[0].nested.count).toBe(100) + // Verify base state unchanged + expect(base.items[0].nested.count).toBe(1) + // Verify result is a copy + expect(result.items[0].nested).not.toBe(base.items[0].nested) + }) + + test("map with transformation then mutate", () => { + const base = createTestData() + const result = produce(base, draft => { + const mapped = draft.items.map(item => ({ + ...item, + doubled: item.value * 2 + })) + // This creates new objects, so mutations won't affect original + mapped[0].value = 999 + }) + expect(result.items[0].value).toBe(10) // Unchanged + }) + + test("map returning original items allows mutation", () => { + const base = createTestData() + const result = produce(base, draft => { + const mapped = draft.items.map(item => item) // Identity map + // Verify mapped items are drafts + expect(isDraft(mapped[0])).toBe(true) + mapped[0].value = 999 + }) + expect(result.items[0].value).toBe(999) + // Verify base state unchanged + expect(base.items[0].value).toBe(10) + // Verify result is a copy + expect(result.items[0]).not.toBe(base.items[0]) + }) + }) + + describe("find()", () => { + test("returns found item", () => { + const base = createTestData() + const result = produce(base, draft => { + const found = draft.items.find(item => item.id === 3) + expect(found?.value).toBe(30) + }) + expect(result).toBe(base) + }) + + test("mutations to found item are reflected", () => { + const base = createTestData() + const result = produce(base, draft => { + const found = draft.items.find(item => item.id === 3) + // Verify found item is a draft + expect(isDraft(found)).toBe(true) + if (found) { + found.value = 999 + } + }) + expect(result.items[2].value).toBe(999) + // Verify base state unchanged + expect(base.items[2].value).toBe(30) + // Verify result is a copy + expect(result.items[2]).not.toBe(base.items[2]) + }) + + test("nested mutations on found item", () => { + const base = createTestData() + const result = produce(base, draft => { + const found = draft.items.find(item => item.id === 2) + // Verify found item is a draft + expect(isDraft(found)).toBe(true) + if (found) { + found.nested.count = 200 + } + }) + expect(result.items[1].nested.count).toBe(200) + // Verify base state unchanged + expect(base.items[1].nested.count).toBe(2) + // Verify result is a copy + expect(result.items[1].nested).not.toBe(base.items[1].nested) + }) + + test("find returns undefined when not found", () => { + const base = createTestData() + const result = produce(base, draft => { + const found = draft.items.find(item => item.id === 999) + expect(found).toBeUndefined() + }) + expect(result).toBe(base) + }) + }) + + describe("findLast()", () => { + test("returns last matching item", () => { + const base = { + items: [ + {id: 1, type: "A"}, + {id: 2, type: "B"}, + {id: 3, type: "A"} + ] + } + const result = produce(base, draft => { + const found = draft.items.findLast(item => item.type === "A") + expect(found?.id).toBe(3) + }) + expect(result).toBe(base) + }) + + test("mutations to findLast result work", () => { + const base = { + items: [ + {id: 1, type: "A", value: 10}, + {id: 2, type: "B", value: 20}, + {id: 3, type: "A", value: 30} + ] + } + const result = produce(base, draft => { + const found = draft.items.findLast(item => item.type === "A") + // Verify found item is a draft + expect(isDraft(found)).toBe(true) + if (found) { + found.value = 999 + } + }) + expect(result.items[2].value).toBe(999) + // Verify base state unchanged + expect(base.items[2].value).toBe(30) + // Verify result is a copy + expect(result.items[2]).not.toBe(base.items[2]) + }) + }) + + describe("slice()", () => { + test("returns sliced array", () => { + const base = createTestData() + const result = produce(base, draft => { + const sliced = draft.items.slice(1, 3) + expect(sliced).toHaveLength(2) + expect(sliced[0].id).toBe(2) + }) + expect(result).toBe(base) + }) + + test("mutations to sliced items work", () => { + const base = createTestData() + const result = produce(base, draft => { + const sliced = draft.items.slice(1, 3) + // Verify sliced items are drafts + expect(isDraft(sliced[0])).toBe(true) + sliced[0].value = 999 + }) + expect(result.items[1].value).toBe(999) + // Verify base state unchanged + expect(base.items[1].value).toBe(20) + // Verify result is a copy + expect(result.items[1]).not.toBe(base.items[1]) + }) + + test("slice with negative indices", () => { + const base = createTestData() + const result = produce(base, draft => { + const sliced = draft.items.slice(-2) + // Verify sliced items are drafts + expect(isDraft(sliced[0])).toBe(true) + sliced[0].value = 999 + }) + expect(result.items[3].value).toBe(999) + // Verify base state unchanged + expect(base.items[3].value).toBe(40) + // Verify result is a copy + expect(result.items[3]).not.toBe(base.items[3]) + }) + }) + + describe("flatMap()", () => { + test("returns flattened array", () => { + const base = { + groups: [{items: [{id: 1}, {id: 2}]}, {items: [{id: 3}, {id: 4}]}] + } + const result = produce(base, draft => { + const flat = draft.groups.flatMap(group => group.items) + expect(flat).toHaveLength(4) + expect(flat[0].id).toBe(1) + }) + expect(result).toBe(base) + }) + + test("mutations to flatMapped items work", () => { + const base = { + groups: [ + { + items: [ + {id: 1, value: 10}, + {id: 2, value: 20} + ] + }, + {items: [{id: 3, value: 30}]} + ] + } + const result = produce(base, draft => { + const flat = draft.groups.flatMap(group => group.items) + // Verify flatMapped items are drafts + expect(isDraft(flat[0])).toBe(true) + flat[0].value = 999 + }) + expect(result.groups[0].items[0].value).toBe(999) + // Verify base state unchanged + expect(base.groups[0].items[0].value).toBe(10) + // Verify result is a copy + expect(result.groups[0].items[0]).not.toBe(base.groups[0].items[0]) + }) + }) + + describe("reduce()", () => { + test("returns accumulated value", () => { + const base = createTestData() + const result = produce(base, draft => { + const sum = draft.items.reduce((acc, item) => acc + item.value, 0) + expect(sum).toBe(150) // 10 + 20 + 30 + 40 + 50 + }) + expect(result).toBe(base) + }) + + test("mutations during reduce are reflected", () => { + const base = createTestData() + const result = produce(base, draft => { + const sum = draft.items.reduce((acc, item, index) => { + // Verify items in reduce are drafts + expect(isDraft(item)).toBe(true) + if (index === 2) { + item.value = 999 + } + return acc + item.value + }, 0) + expect(sum).toBe(1119) // 10 + 20 + 999 + 40 + 50 + }) + expect(result.items[2].value).toBe(999) + // Verify base state unchanged + expect(base.items[2].value).toBe(30) + // Verify result is a copy + expect(result.items[2]).not.toBe(base.items[2]) + }) + + test("reduce with object accumulator", () => { + const base = createTestData() + const result = produce(base, draft => { + const grouped = draft.items.reduce((acc, item) => { + const key = item.value > 25 ? "high" : "low" + if (!acc[key]) acc[key] = [] + acc[key].push(item) + return acc + }, {}) + expect(grouped.low).toHaveLength(2) + expect(grouped.high).toHaveLength(3) + }) + expect(result).toBe(base) + }) + + test("reduce then mutate accumulated items", () => { + const base = createTestData() + const result = produce(base, draft => { + const items = draft.items.reduce((acc, item) => { + // Verify items in reduce are drafts + expect(isDraft(item)).toBe(true) + if (item.value > 25) acc.push(item) + return acc + }, []) + // Verify accumulated items are drafts + expect(isDraft(items[0])).toBe(true) + items[0].value = 999 + }) + expect(result.items[2].value).toBe(999) // id: 3 is at index 2 + // Verify base state unchanged + expect(base.items[2].value).toBe(30) + // Verify result is a copy + expect(result.items[2]).not.toBe(base.items[2]) + }) + }) + + describe("forEach()", () => { + test("iterates over all items", () => { + const base = createTestData() + const result = produce(base, draft => { + let count = 0 + draft.items.forEach(item => { + count++ + expect(item.id).toBeDefined() + }) + expect(count).toBe(5) + }) + expect(result).toBe(base) + }) + + test("mutations during forEach are reflected", () => { + const base = createTestData() + const result = produce(base, draft => { + draft.items.forEach((item, index) => { + // Verify items in forEach are drafts + expect(isDraft(item)).toBe(true) + if (index === 1) { + item.value = 999 + } + }) + }) + expect(result.items[1].value).toBe(999) + // Verify base state unchanged + expect(base.items[1].value).toBe(20) + // Verify result is a copy + expect(result.items[1]).not.toBe(base.items[1]) + }) + + test("forEach with nested mutations", () => { + const base = createTestData() + const result = produce(base, draft => { + draft.items.forEach(item => { + // Verify items in forEach are drafts + expect(isDraft(item)).toBe(true) + item.nested.count *= 10 + }) + }) + expect(result.items[0].nested.count).toBe(10) + expect(result.items[4].nested.count).toBe(50) + // Verify base state unchanged + expect(base.items[0].nested.count).toBe(1) + // Verify result is a copy + expect(result.items[0].nested).not.toBe(base.items[0].nested) + }) + + test("forEach returns undefined", () => { + const base = createTestData() + const result = produce(base, draft => { + const returnValue = draft.items.forEach(item => item.value) + expect(returnValue).toBeUndefined() + }) + expect(result).toBe(base) + }) + }) + + describe("indexOf()", () => { + test("returns index of found item", () => { + const base = {items: [1, 2, 3, 4, 5]} + const result = produce(base, draft => { + const index = draft.items.indexOf(3) + expect(index).toBe(2) + }) + expect(result).toBe(base) + }) + + test("returns -1 when item not found", () => { + const base = {items: [1, 2, 3, 4, 5]} + const result = produce(base, draft => { + const index = draft.items.indexOf(99) + expect(index).toBe(-1) + }) + expect(result).toBe(base) + }) + + test("works with object references", () => { + const base = createTestData() + const result = produce(base, draft => { + const item = draft.items[2] + const index = draft.items.indexOf(item) + expect(index).toBe(2) + }) + expect(result).toBe(base) + }) + + test("indexOf with fromIndex parameter", () => { + const base = {items: [1, 2, 3, 2, 5]} + const result = produce(base, draft => { + const firstIndex = draft.items.indexOf(2) + const secondIndex = draft.items.indexOf(2, 2) + expect(firstIndex).toBe(1) + expect(secondIndex).toBe(3) + }) + expect(result).toBe(base) + }) + }) + + describe("join()", () => { + test("returns joined string", () => { + const base = {items: [1, 2, 3, 4, 5]} + const result = produce(base, draft => { + const joined = draft.items.join(",") + expect(joined).toBe("1,2,3,4,5") + }) + expect(result).toBe(base) + }) + + test("join with custom separator", () => { + const base = {items: ["a", "b", "c"]} + const result = produce(base, draft => { + const joined = draft.items.join(" - ") + expect(joined).toBe("a - b - c") + }) + expect(result).toBe(base) + }) + + test("join with no separator uses comma", () => { + const base = {items: [1, 2, 3]} + const result = produce(base, draft => { + const joined = draft.items.join() + expect(joined).toBe("1,2,3") + }) + expect(result).toBe(base) + }) + + test("join with objects calls toString", () => { + const base = createTestData() + const result = produce(base, draft => { + const joined = draft.items.join(",") + expect(joined).toContain("[object Object]") + }) + expect(result).toBe(base) + }) + }) + + describe("combined operations", () => { + test("chain filter then map then mutate", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 20) + // Verify filtered items are drafts + filtered.forEach(item => expect(isDraft(item)).toBe(true)) + const nested = filtered.map(item => item.nested) + // Verify mapped items are drafts + expect(isDraft(nested[0])).toBe(true) + nested[0].count = 999 + }) + expect(result.items[2].nested.count).toBe(999) + // Verify base state unchanged + expect(base.items[2].nested.count).toBe(3) + // Verify result is a copy + expect(result.items[2].nested).not.toBe(base.items[2].nested) + }) + + test("filter, find, then mutate", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 20) + // Verify filtered items are drafts + filtered.forEach(item => expect(isDraft(item)).toBe(true)) + const found = filtered.find(item => item.id === 4) + // Verify found item is a draft + expect(isDraft(found)).toBe(true) + if (found) { + found.value = 999 + } + }) + expect(result.items[3].value).toBe(999) + // Verify base state unchanged + expect(base.items[3].value).toBe(40) + // Verify result is a copy + expect(result.items[3]).not.toBe(base.items[3]) + }) + + test("multiple filters with mutations", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered1 = draft.items.filter(item => item.value > 15) + // Verify first filter items are drafts + filtered1.forEach(item => expect(isDraft(item)).toBe(true)) + const filtered2 = filtered1.filter(item => item.value < 45) + // Verify second filter items are drafts + filtered2.forEach(item => expect(isDraft(item)).toBe(true)) + filtered2[0].value = 999 + }) + expect(result.items[1].value).toBe(999) + // Verify base state unchanged + expect(base.items[1].value).toBe(20) + // Verify result is a copy + expect(result.items[1]).not.toBe(base.items[1]) + }) + }) + + describe("edge cases", () => { + test("empty filter result", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 1000) + expect(filtered).toHaveLength(0) + }) + expect(result).toBe(base) + }) + + test("filter with all items", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(() => true) + filtered[0].value = 999 + }) + expect(result.items[0].value).toBe(999) + }) + + test("mutation during filter callback", () => { + const base = createTestData() + const result = produce(base, draft => { + const filtered = draft.items.filter(item => { + // Verify items in filter callback are drafts + expect(isDraft(item)).toBe(true) + item.touched = true + return item.value > 25 + }) + expect(filtered).toHaveLength(3) + }) + expect(result.items[0].touched).toBe(true) + expect(result.items[3].touched).toBe(true) + // Verify base state unchanged + expect(base.items[0].touched).toBeUndefined() + }) + + test("primitive array filter", () => { + const base = {numbers: [1, 2, 3, 4, 5]} + const result = produce(base, draft => { + const filtered = draft.numbers.filter(n => n > 2) + expect(filtered).toEqual([3, 4, 5]) + }) + expect(result).toBe(base) + }) + + test("mixed draft and non-draft in filter result", () => { + const base = createTestData() + const result = produce(base, draft => { + // Mutate some items before filtering + draft.items[0].value = 15 + draft.items[2].value = 35 + + const filtered = draft.items.filter(item => item.value > 14) + filtered[0].value = 999 // Mutate already-mutated item + }) + expect(result.items[0].value).toBe(999) + }) + + test("assigning filtered array back to draft", () => { + const base = createTestData() + const result = produce(base, draft => { + draft.items = draft.items.filter(item => item.value > 25) + draft.items[0].value = 999 + }) + expect(result.items).toHaveLength(3) + expect(result.items[0].value).toBe(999) + expect(result.items[0].id).toBe(3) + }) + }) + + describe("performance-critical patterns", () => { + test("large array filter with read-only access", () => { + const base = { + items: Array.from({length: 1000}, (_, i) => ({ + id: i, + value: i * 10 + })) + } + const result = produce(base, draft => { + const filtered = draft.items.filter(item => item.value > 5000) + const sum = filtered.reduce((acc, item) => acc + item.value, 0) + expect(sum).toBeGreaterThan(0) + }) + expect(result).toBe(base) // No modifications + }) + + test("multiple filters without mutations", () => { + const base = createTestData() + const result = produce(base, draft => { + const f1 = draft.items.filter(item => item.value > 10) + const f2 = draft.items.filter(item => item.value < 40) + const f3 = draft.items.filter(item => item.id % 2 === 0) + expect(f1.length + f2.length + f3.length).toBeGreaterThan(0) + }) + expect(result).toBe(base) + }) }) - expect(nextState).not.toBe(baseState) - expect(nextState).toEqual([1, 2, 3]) + }) + + it("supports the same child reference multiple times in the same array via index assignment", () => { + const obj = {value: 1} + const baseState = {items: [obj, {}, {}, {}, {}]} + + const nextState = produce(baseState, draft => { + // Assign the same object to multiple indices + draft.items[0] = obj // Original position + draft.items[2] = obj // Same object at different index + draft.items[4] = obj // Same object at yet another index + + // Modify the object through one of the references + draft.items[0].value = 2 + }) + + // Immer behavior: modified draft gets new object, unmodified drafts are optimized + expect(nextState.items[0]).not.toBe(nextState.items[2]) // Modified vs unmodified + expect(nextState.items[2]).toBe(nextState.items[4]) // Both unmodified, same reference + expect(nextState.items[0].value).toBe(2) // Modified + expect(nextState.items[2].value).toBe(1) // Unmodified (same as original) + expect(nextState.items[4].value).toBe(1) // Unmodified (same as original) + + // The unmodified items should be the same as the original object + expect(nextState.items[2]).toBe(obj) + expect(nextState.items[4]).toBe(obj) + + // Original object should be unchanged + expect(obj.value).toBe(1) + + // Verify array structure + expect(nextState.items.length).toBe(5) }) it("supports modifying nested objects", () => { const baseState = [{a: 1}, {}] const nextState = produce(baseState, s => { + s[0].a++ s[0].a++ s[1].a = 0 + s[0].a-- }) expect(nextState).not.toBe(baseState) expect(nextState[0].a).toBe(2) @@ -1082,7 +2129,7 @@ function runBaseTest(name, autoFreeze, useStrictShallowCopy, useListener) { }) it("supports a base state with multiple references to an object", () => { - const obj = {} + const obj = {notEmpty: true} const res = produce({a: obj, b: obj}, d => { // Two drafts are created for each occurrence of an object in the base state. expect(d.a).not.toBe(d.b) @@ -1709,6 +2756,7 @@ function runBaseTest(name, autoFreeze, useStrictShallowCopy, useListener) { expect(next[0]).toBe(next[1]) }) + // This actually seems to pass now! it("cannot return an object that references itself", () => { const res = {} res.self = res From 773455af1e6281d6b335f7804efa55d7e1e52c4e Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Mon, 6 Oct 2025 17:22:47 +0200 Subject: [PATCH 2/2] Add update scenario tests --- __tests__/updateScenarios.js | 557 +++++++++++++++++++++++++++++++++++ 1 file changed, 557 insertions(+) create mode 100644 __tests__/updateScenarios.js diff --git a/__tests__/updateScenarios.js b/__tests__/updateScenarios.js new file mode 100644 index 00000000..83cac47b --- /dev/null +++ b/__tests__/updateScenarios.js @@ -0,0 +1,557 @@ +import {describe, test, expect, beforeEach} from "vitest" +import {produce} from "../src/immer" + +// Test configuration - smaller values for faster tests +const TEST_CONFIG = { + arraySize: 10, + nestedArraySize: 2, + multiUpdateCount: 5, + reuseStateIterations: 5 +} + +function createInitialState(arraySize = TEST_CONFIG.arraySize) { + const initialState = { + largeArray: Array.from({length: arraySize}, (_, i) => ({ + id: i, + value: Math.random(), + nested: {key: `key-${i}`, data: Math.random()}, + moreNested: { + items: Array.from({length: TEST_CONFIG.nestedArraySize}, (_, i) => ({ + id: i, + name: String(i) + })) + } + })), + otherData: Array.from({length: arraySize}, (_, i) => ({ + id: i, + name: `name-${i}`, + isActive: i % 2 === 0 + })) + } + return initialState +} + +// Utility functions for calculating array indices based on size +const getValidIndex = (arraySize = TEST_CONFIG.arraySize) => { + // Return a valid index (not the last one to avoid edge cases) + return Math.min(arraySize - 2, Math.max(0, arraySize - 2)) +} + +const getValidId = (arraySize = TEST_CONFIG.arraySize) => { + // Return a valid ID that exists in the array + return Math.min(arraySize - 2, Math.max(0, arraySize - 2)) +} + +const getHighIndex = (offset = 0, arraySize = TEST_CONFIG.arraySize) => { + // Calculate high index (80% through the array + offset) + return Math.floor(arraySize * 0.8) + (offset % Math.floor(arraySize * 0.2)) +} + +// Action creators +const add = index => ({ + type: "test/addItem", + payload: {id: index, value: index, nested: {data: index}} +}) + +const remove = index => ({type: "test/removeItem", payload: index}) + +const filter = percentToKeep => ({ + type: "test/filterItem", + payload: percentToKeep +}) + +const update = index => ({ + type: "test/updateItem", + payload: {id: index, value: index, nestedData: index} +}) + +const concat = index => ({ + type: "test/concatArray", + payload: Array.from({length: 50}, (_, i) => ({id: i, value: index})) +}) + +const updateHigh = index => ({ + type: "test/updateHighIndex", + payload: { + id: getHighIndex(index), + value: index, + nestedData: index + } +}) + +const updateMultiple = index => ({ + type: "test/updateMultiple", + payload: Array.from({length: TEST_CONFIG.multiUpdateCount}, (_, i) => ({ + id: (index + i) % TEST_CONFIG.arraySize, + value: index + i, + nestedData: index + i + })) +}) + +const removeHigh = index => ({ + type: "test/removeHighIndex", + payload: getHighIndex(index) +}) + +const sortByIdReverse = () => ({ + type: "test/sortByIdReverse" +}) + +const reverseArray = () => ({ + type: "test/reverseArray" +}) + +const actions = { + add, + remove, + filter, + update, + concat, + updateHigh, + updateMultiple, + removeHigh, + sortByIdReverse, + reverseArray +} + +// Vanilla reducer for comparison +const vanillaReducer = (state = createInitialState(), action) => { + switch (action.type) { + case "test/addItem": + return { + ...state, + largeArray: [...state.largeArray, action.payload] + } + case "test/removeItem": { + const newArray = state.largeArray.slice() + newArray.splice(action.payload, 1) + return { + ...state, + largeArray: newArray + } + } + case "test/filterItem": { + const length = state.largeArray.length + const newArray = state.largeArray.filter( + (item, i) => i / length < action.payload + ) + return { + ...state, + largeArray: newArray + } + } + case "test/updateItem": { + return { + ...state, + largeArray: state.largeArray.map(item => + item.id === action.payload.id + ? { + ...item, + value: action.payload.value, + nested: {...item.nested, data: action.payload.nestedData} + } + : item + ) + } + } + case "test/concatArray": { + const length = state.largeArray.length + const newArray = action.payload.concat(state.largeArray) + newArray.length = length + return { + ...state, + largeArray: newArray + } + } + case "test/updateHighIndex": { + return { + ...state, + largeArray: state.largeArray.map(item => + item.id === action.payload.id + ? { + ...item, + value: action.payload.value, + nested: {...item.nested, data: action.payload.nestedData} + } + : item + ) + } + } + case "test/updateMultiple": { + const updates = new Map(action.payload.map(p => [p.id, p])) + return { + ...state, + largeArray: state.largeArray.map(item => { + const update = updates.get(item.id) + return update + ? { + ...item, + value: update.value, + nested: {...item.nested, data: update.nestedData} + } + : item + }) + } + } + case "test/removeHighIndex": { + const newArray = state.largeArray.slice() + const indexToRemove = newArray.findIndex( + item => item.id === action.payload + ) + if (indexToRemove !== -1) { + newArray.splice(indexToRemove, 1) + } + return { + ...state, + largeArray: newArray + } + } + case "test/sortByIdReverse": { + const newArray = state.largeArray.slice() + newArray.sort((a, b) => b.id - a.id) // Sort by ID in reverse order + return { + ...state, + largeArray: newArray + } + } + case "test/reverseArray": { + const newArray = state.largeArray.slice() + newArray.reverse() + return { + ...state, + largeArray: newArray + } + } + default: + return state + } +} + +// Immer reducer +const createImmerReducer = produce => { + const immerReducer = (state = createInitialState(), action) => + produce(state, draft => { + switch (action.type) { + case "test/addItem": + draft.largeArray.push(action.payload) + break + case "test/removeItem": + draft.largeArray.splice(action.payload, 1) + break + case "test/filterItem": { + const keepPercentage = action.payload / 10 + const length = state.largeArray.length + draft.largeArray = draft.largeArray.filter( + (item, i) => i / length < action.payload + ) + break + } + case "test/updateItem": { + const item = draft.largeArray.find( + item => item.id === action.payload.id + ) + if (item) { + item.value = action.payload.value + item.nested.data = action.payload.nestedData + } + break + } + case "test/concatArray": { + const length = state.largeArray.length + const newArray = action.payload.concat(state.largeArray) + newArray.length = length + draft.largeArray = newArray + break + } + case "test/updateHighIndex": { + const item = draft.largeArray.find( + item => item.id === action.payload.id + ) + if (item) { + item.value = action.payload.value + item.nested.data = action.payload.nestedData + } + break + } + case "test/updateMultiple": { + action.payload.forEach(update => { + const item = draft.largeArray.find(item => item.id === update.id) + if (item) { + item.value = update.value + item.nested.data = update.nestedData + } + }) + break + } + case "test/removeHighIndex": { + const indexToRemove = draft.largeArray.findIndex( + item => item.id === action.payload + ) + if (indexToRemove !== -1) { + draft.largeArray.splice(indexToRemove, 1) + } + break + } + case "test/sortByIdReverse": { + draft.largeArray.sort((a, b) => b.id - a.id) + break + } + case "test/reverseArray": { + draft.largeArray.reverse() + break + } + } + }) + + return immerReducer +} + +const immerReducer = createImmerReducer(produce) + +describe("Update Scenarios - Single Operations", () => { + let initialState + + beforeEach(() => { + initialState = createInitialState() + }) + + test("add scenario", () => { + const action = actions.add(0) + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + expect(immerResult.largeArray.length).toBe(vanillaResult.largeArray.length) + expect(immerResult.largeArray[immerResult.largeArray.length - 1]).toEqual( + action.payload + ) + }) + + test("remove scenario", () => { + const action = actions.remove(getValidIndex()) + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + expect(immerResult.largeArray.length).toBe(vanillaResult.largeArray.length) + expect(immerResult.largeArray.length).toBe( + initialState.largeArray.length - 1 + ) + }) + + test("filter scenario", () => { + // Keep 60% of the items + const percentage = 0.6 + const action = actions.filter(percentage) + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + expect(immerResult.largeArray.length).toBe(vanillaResult.largeArray.length) + expect(immerResult.largeArray.length).toBe( + initialState.largeArray.length * percentage + ) + }) + + test("update scenario", () => { + const targetId = getValidId() + const action = actions.update(targetId) + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + const updatedItem = immerResult.largeArray.find( + item => item.id === targetId + ) + expect(updatedItem.value).toBe(targetId) + expect(updatedItem.nested.data).toBe(targetId) + }) + + test("concat scenario", () => { + const action = actions.concat(1) + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + expect(immerResult.largeArray.length).toBe(vanillaResult.largeArray.length) + expect(immerResult.largeArray.length).toBe(initialState.largeArray.length) + }) + + test("updateHigh scenario", () => { + const action = actions.updateHigh(2) + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + const targetId = action.payload.id + const updatedItem = immerResult.largeArray.find( + item => item.id === targetId + ) + expect(updatedItem.value).toBe(2) + expect(updatedItem.nested.data).toBe(2) + }) + + test("updateMultiple scenario", () => { + const action = actions.updateMultiple(3) + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + action.payload.forEach(update => { + const updatedItem = immerResult.largeArray.find( + item => item.id === update.id + ) + expect(updatedItem.value).toBe(update.value) + expect(updatedItem.nested.data).toBe(update.nestedData) + }) + }) + + test("removeHigh scenario", () => { + const action = actions.removeHigh(1) + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + expect(immerResult.largeArray.length).toBe(vanillaResult.largeArray.length) + const removedItem = immerResult.largeArray.find( + item => item.id === action.payload + ) + expect(removedItem).toBeUndefined() + }) + + test("sortByIdReverse scenario", () => { + const action = actions.sortByIdReverse() + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + expect(immerResult.largeArray.length).toBe(vanillaResult.largeArray.length) + expect(immerResult.largeArray.length).toBe(initialState.largeArray.length) + + // Verify sorting - first element should have highest ID + expect(immerResult.largeArray[0].id).toBe(TEST_CONFIG.arraySize - 1) + expect(immerResult.largeArray[immerResult.largeArray.length - 1].id).toBe(0) + + // Verify arrays match + expect(immerResult.largeArray.map(item => item.id)).toEqual( + vanillaResult.largeArray.map(item => item.id) + ) + }) + + test("reverseArray scenario", () => { + const action = actions.reverseArray() + const vanillaResult = vanillaReducer(initialState, action) + const immerResult = immerReducer(initialState, action) + + expect(immerResult.largeArray.length).toBe(vanillaResult.largeArray.length) + expect(immerResult.largeArray.length).toBe(initialState.largeArray.length) + + // Verify reversal - first element should be what was last + expect(immerResult.largeArray[0].id).toBe(TEST_CONFIG.arraySize - 1) + expect(immerResult.largeArray[immerResult.largeArray.length - 1].id).toBe(0) + + // Verify arrays match + expect(immerResult.largeArray.map(item => item.id)).toEqual( + vanillaResult.largeArray.map(item => item.id) + ) + }) +}) + +describe("Update Scenarios - State Reuse", () => { + test("update-reuse scenario", () => { + let currentState = createInitialState() + + for (let i = 0; i < TEST_CONFIG.reuseStateIterations; i++) { + currentState = immerReducer(currentState, actions.update(i)) + } + + // Verify the final state has all updates applied + for (let i = 0; i < TEST_CONFIG.reuseStateIterations; i++) { + const item = currentState.largeArray.find(item => item.id === i) + expect(item.value).toBe(i) + expect(item.nested.data).toBe(i) + } + }) + + test("updateHigh-reuse scenario", () => { + let currentState = createInitialState() + + for (let i = 0; i < TEST_CONFIG.reuseStateIterations; i++) { + currentState = immerReducer(currentState, actions.updateHigh(i)) + } + + // Verify updates were applied to high indices + expect(currentState.largeArray.length).toBe(TEST_CONFIG.arraySize) + }) + + test("remove-reuse scenario", () => { + let currentState = createInitialState() + const originalLength = currentState.largeArray.length + + for (let i = 0; i < TEST_CONFIG.reuseStateIterations; i++) { + currentState = immerReducer(currentState, actions.remove(0)) // Always remove first item + } + + expect(currentState.largeArray.length).toBe( + originalLength - TEST_CONFIG.reuseStateIterations + ) + }) + + test("removeHigh-reuse scenario", () => { + let currentState = createInitialState() + const originalLength = currentState.largeArray.length + + for (let i = 0; i < TEST_CONFIG.reuseStateIterations; i++) { + currentState = immerReducer(currentState, actions.removeHigh(i)) + } + + expect(currentState.largeArray.length).toBeLessThan(originalLength) + }) +}) + +describe("Update Scenarios - Mixed Sequence", () => { + test("mixed-sequence scenario", () => { + let state = createInitialState() + const originalLength = state.largeArray.length + + // Perform a sequence of different operations (typical workflow) + state = immerReducer(state, actions.add(1)) + expect(state.largeArray.length).toBe(originalLength + 1) + + const targetId = getValidId() + state = immerReducer(state, actions.update(targetId)) + const updatedItem = state.largeArray.find(item => item.id === targetId) + expect(updatedItem.value).toBe(targetId) + + state = immerReducer(state, actions.updateHigh(2)) + state = immerReducer(state, actions.updateMultiple(3)) + + state = immerReducer(state, actions.remove(getValidIndex())) + expect(state.largeArray.length).toBe(originalLength) // +1 from add, -1 from remove + + // Verify final state integrity + expect(state.largeArray).toBeDefined() + expect(state.otherData).toBeDefined() + expect(state.largeArray.every(item => item.id !== undefined)).toBe(true) + }) +}) + +describe("Update Scenarios - Performance Focused", () => { + test("large array operations", () => { + const largeState = createInitialState(5000) + + // Test that operations complete without timeout + const result1 = immerReducer(largeState, actions.update(1000)) + expect(result1.largeArray.length).toBe(5000) + + const result2 = immerReducer(largeState, actions.updateHigh(10)) + expect(result2.largeArray.length).toBe(5000) + + const result3 = immerReducer(largeState, actions.updateMultiple(20)) + expect(result3.largeArray.length).toBe(5000) + }) + + test("nested data modifications", () => { + const state = createInitialState(100) + + const result = immerReducer(state, actions.update(50)) + const updatedItem = result.largeArray.find(item => item.id === 50) + + expect(updatedItem.nested.data).toBe(50) + expect(updatedItem.moreNested.items).toBeDefined() + expect(updatedItem.moreNested.items.length).toBe( + TEST_CONFIG.nestedArraySize + ) + }) +})