From e19afbd8bddbadb97b98adfe022a5a35a714dde9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Nov 2025 20:42:08 +0000 Subject: [PATCH 01/13] test: add regression tests for createOptimisticAction with syncMode on-demand Add comprehensive tests to verify that createOptimisticAction works correctly with syncMode "on-demand" collections. These tests were created while investigating a reported bug where mutationFn was not being called when using on-demand collections. Tests added: - Basic on-demand collection with insert - Collection not started (idle state) - Collection in loading state (not ready) - On-demand collection with live query filter - UPDATE operation on on-demand collection - Debug test verifying mutations array is populated All tests pass, suggesting the reported issue may have been fixed by recent scheduler bug fixes or is specific to a configuration not covered here. --- packages/db/tests/optimistic-action.test.ts | 352 ++++++++++++++++++++ 1 file changed, 352 insertions(+) diff --git a/packages/db/tests/optimistic-action.test.ts b/packages/db/tests/optimistic-action.test.ts index 95f43c242..0fb286cb9 100644 --- a/packages/db/tests/optimistic-action.test.ts +++ b/packages/db/tests/optimistic-action.test.ts @@ -1,5 +1,6 @@ import { describe, expect, expectTypeOf, it, vi } from "vitest" import { createCollection, createOptimisticAction } from "../src" +import { createLiveQueryCollection, eq } from "../src/query" import type { MutationFnParams, Transaction, @@ -207,6 +208,357 @@ describe(`createOptimisticAction`, () => { expectTypeOf(userAction).returns.toEqualTypeOf() }) + // Test with syncMode "on-demand" + it(`should call mutationFn when using syncMode on-demand`, async () => { + // This test reproduces the bug where mutationFn is not called + // when the collection is configured with syncMode: "on-demand" + // Bug report: https://discord.com/channels/... + // - onMutate runs but mutationFn never runs + // - works with eager mode but not on-demand + + const onMutateMock = vi.fn() + const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) + + // Create a collection with syncMode: "on-demand" + // This requires a loadSubset handler + const collection = createCollection<{ id: string; text: string }>({ + id: `on-demand-collection`, + getKey: (item) => item.id, + syncMode: `on-demand`, + sync: { + sync: ({ markReady }) => { + // For on-demand mode, we mark ready immediately but don't load data + // Data is loaded on-demand via loadSubset + markReady() + + return { + loadSubset: () => { + // No-op for testing - just return true to indicate sync + return true + }, + } + }, + }, + }) + + // Create an optimistic action + const addTodo = createOptimisticAction({ + onMutate: (text) => { + collection.insert({ id: `1`, text }) + onMutateMock(text) + }, + mutationFn: async (text, params) => { + return Promise.resolve(mutationFnMock(text, params)) + }, + }) + + // Execute the optimistic action + const transaction = addTodo(`Test Todo`) + + // Verify onMutate was called + expect(onMutateMock).toHaveBeenCalledWith(`Test Todo`) + + // Verify the optimistic update was applied to the collection + expect(collection.get(`1`)).toEqual({ id: `1`, text: `Test Todo` }) + + // Wait for the mutation to complete + await transaction.isPersisted.promise + + // BUG: mutationFn should be called but it's not! + expect(mutationFnMock).toHaveBeenCalledTimes(1) + }) + + // Test with syncMode "on-demand" where collection has NOT started sync yet + it(`should call mutationFn when collection is not started (idle)`, async () => { + // This test checks if mutationFn is called when the collection hasn't started sync + const onMutateMock = vi.fn() + const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) + + // Create a collection that doesn't start sync automatically + const collection = createCollection<{ id: string; text: string }>({ + id: `idle-collection`, + getKey: (item) => item.id, + startSync: false, + sync: { + sync: () => { + // No-op sync for testing + }, + }, + }) + + // Create an optimistic action + const addTodo = createOptimisticAction({ + onMutate: (text) => { + collection.insert({ id: `1`, text }) + onMutateMock(text) + }, + mutationFn: async (text, params) => { + return Promise.resolve(mutationFnMock(text, params)) + }, + }) + + // Execute the optimistic action (collection is in idle state) + const transaction = addTodo(`Test Todo`) + + // Verify onMutate was called + expect(onMutateMock).toHaveBeenCalledWith(`Test Todo`) + + // Verify the optimistic update was applied + expect(collection.get(`1`)).toEqual({ id: `1`, text: `Test Todo` }) + + // Wait for the mutation to complete + await transaction.isPersisted.promise + + // mutationFn should be called + expect(mutationFnMock).toHaveBeenCalledTimes(1) + }) + + // Test with syncMode "on-demand" where sync is in loading state (not ready yet) + it(`should call mutationFn when collection is loading (not ready)`, async () => { + // This test checks if mutationFn is called when the collection is still loading + const onMutateMock = vi.fn() + const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) + + // Create a collection that starts sync but doesn't call markReady + const collection = createCollection<{ id: string; text: string }>({ + id: `loading-collection`, + getKey: (item) => item.id, + startSync: true, + sync: { + sync: () => { + // Intentionally don't call markReady - collection stays in "loading" state + return () => {} + }, + }, + }) + + // Create an optimistic action + const addTodo = createOptimisticAction({ + onMutate: (text) => { + collection.insert({ id: `1`, text }) + onMutateMock(text) + }, + mutationFn: async (text, params) => { + return Promise.resolve(mutationFnMock(text, params)) + }, + }) + + // Execute the optimistic action (collection is in loading state) + const transaction = addTodo(`Test Todo`) + + // Verify onMutate was called + expect(onMutateMock).toHaveBeenCalledWith(`Test Todo`) + + // Verify the optimistic update was applied + expect(collection.get(`1`)).toEqual({ id: `1`, text: `Test Todo` }) + + // Wait for the mutation to complete + await transaction.isPersisted.promise + + // mutationFn should be called + expect(mutationFnMock).toHaveBeenCalledTimes(1) + }) + + // Test with on-demand collection and a live query with filters - the reported scenario + it(`should call mutationFn with on-demand collection and live query filter`, async () => { + // This test attempts to reproduce the exact bug scenario: + // - Base collection with syncMode: "on-demand" + // - Live query collection with filters on top + // - createOptimisticAction used to mutate the base collection + // - Bug: onMutate runs but mutationFn never runs + + const onMutateMock = vi.fn() + const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) + + // Create a base collection with syncMode: "on-demand" + type Todo = { id: string; text: string; status: string } + const baseCollection = createCollection({ + id: `on-demand-base-collection`, + getKey: (item) => item.id, + syncMode: `on-demand`, + startSync: true, + sync: { + sync: ({ markReady, begin, write, commit }) => { + // Simulate on-demand mode: mark ready immediately, load data on demand + markReady() + + // Pre-populate with some data + begin() + write({ + type: `insert`, + value: { id: `1`, text: `Existing todo`, status: `active` }, + }) + commit() + + return { + loadSubset: () => { + return true + }, + } + }, + }, + }) + + // Create a live query collection with a filter on status + const activeTodos = createLiveQueryCollection({ + id: `active-todos-query`, + startSync: true, + query: (q) => + q + .from({ todo: baseCollection }) + .where(({ todo }) => eq(todo.status, `active`)) + .select(({ todo }) => ({ todo })), + }) + + // Wait for the live query to be ready + await activeTodos.preload() + + // Verify initial state + expect([...activeTodos.values()].length).toBe(1) + expect([...activeTodos.values()][0]?.todo.text).toBe(`Existing todo`) + + // Create an optimistic action to INSERT a new todo + const addTodo = createOptimisticAction<{ text: string }>({ + onMutate: (input) => { + baseCollection.insert({ id: `2`, text: input.text, status: `active` }) + onMutateMock(input) + }, + mutationFn: async (input, params) => { + return Promise.resolve(mutationFnMock(input, params)) + }, + }) + + // Execute the optimistic action + const transaction = addTodo({ text: `New todo` }) + + // Verify onMutate was called + expect(onMutateMock).toHaveBeenCalledWith({ text: `New todo` }) + + // Verify the optimistic update was applied to the base collection + expect(baseCollection.get(`2`)).toEqual({ + id: `2`, + text: `New todo`, + status: `active`, + }) + + // Wait for the mutation to complete + await transaction.isPersisted.promise + + // BUG: mutationFn should be called! + expect(mutationFnMock).toHaveBeenCalledTimes(1) + }) + + // Test UPDATE scenario which might have different behavior + it(`should call mutationFn when UPDATE is performed on on-demand collection`, async () => { + const onMutateMock = vi.fn() + const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) + + type Todo = { id: string; text: string; status: string } + const collection = createCollection({ + id: `on-demand-update-collection`, + getKey: (item) => item.id, + syncMode: `on-demand`, + startSync: true, + sync: { + sync: ({ markReady, begin, write, commit }) => { + markReady() + + // Pre-populate with data + begin() + write({ + type: `insert`, + value: { id: `1`, text: `Original text`, status: `active` }, + }) + commit() + + return { + loadSubset: () => true, + } + }, + }, + }) + + // Create an optimistic action to UPDATE an existing todo + const updateTodo = createOptimisticAction<{ id: string; text: string }>({ + onMutate: (input) => { + collection.update(input.id, (draft) => { + draft.text = input.text + }) + onMutateMock(input) + }, + mutationFn: async (input, params) => { + return Promise.resolve(mutationFnMock(input, params)) + }, + }) + + // Execute the optimistic action + const transaction = updateTodo({ id: `1`, text: `Updated text` }) + + // Verify onMutate was called + expect(onMutateMock).toHaveBeenCalledWith({ id: `1`, text: `Updated text` }) + + // Verify the optimistic update was applied + expect(collection.get(`1`)?.text).toBe(`Updated text`) + + // Wait for the mutation to complete + await transaction.isPersisted.promise + + // mutationFn should be called + expect(mutationFnMock).toHaveBeenCalledTimes(1) + }) + + // Debug test: verify mutations array is populated correctly + it(`should have mutations in the transaction after onMutate completes`, async () => { + const onMutateMock = vi.fn() + const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) + + // Create an on-demand collection with live query filter + type Todo = { id: string; text: string; status: string } + const baseCollection = createCollection({ + id: `debug-collection`, + getKey: (item) => item.id, + syncMode: `on-demand`, + startSync: true, + sync: { + sync: ({ markReady }) => { + markReady() + return { + loadSubset: () => true, + } + }, + }, + }) + + // Track mutations count at mutationFn call time + let mutationsAtMutationFn: number | undefined + + const addTodo = createOptimisticAction<{ text: string }>({ + onMutate: (input) => { + baseCollection.insert({ id: `1`, text: input.text, status: `active` }) + onMutateMock(input) + }, + mutationFn: async (input, params) => { + // Record the number of mutations at this point + mutationsAtMutationFn = params.transaction.mutations.length + return Promise.resolve(mutationFnMock(input, params)) + }, + }) + + const transaction = addTodo({ text: `Test` }) + + // Verify onMutate was called + expect(onMutateMock).toHaveBeenCalled() + + // Wait for the transaction to complete + await transaction.isPersisted.promise + + // Verify mutationFn was called + expect(mutationFnMock).toHaveBeenCalledTimes(1) + + // Verify there was at least one mutation + expect(mutationsAtMutationFn).toBeGreaterThan(0) + }) + // Test error handling it(`should handle errors in mutationFn correctly`, async () => { // Setup a mock collection From 2d979a07b8a0360743e1ba87635e38d2b1b76dc3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Nov 2025 21:26:00 +0000 Subject: [PATCH 02/13] test: add failing tests for array iteration change tracking These tests reproduce the bug where modifications to array items retrieved via iteration methods (find(), forEach, for...of) are not tracked by the change proxy. Root cause: When using array iteration methods like find(), the proxy returns raw array elements instead of proxied versions. This causes mutations to these elements to not be tracked, resulting in getChanges() returning an empty object. This directly causes the createOptimisticAction bug where mutationFn is never called when modifying nested array items via: draft.nested.array.find(...).property = value Failing tests: - find() returns unproxied items - forEach() iterates over unproxied items - for...of iterates over unproxied items Working test: - Direct index access [0] works because the proxy's get handler intercepts numeric index access and returns proxied items The fix requires handling array iteration methods similar to how Map/Set iteration is already handled in the proxy. --- packages/db/tests/proxy.test.ts | 83 +++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index f063d43a7..33bf2cd05 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -1653,5 +1653,88 @@ describe(`Proxy Library`, () => { }) expect(obj.date).toEqual(originalDate) }) + + // BUG: Array.find() returns unproxied items, so changes to them aren't tracked + // This is the root cause of the createOptimisticAction bug where mutationFn + // is never called when modifying nested array items via .find() + it(`should track changes when modifying array items retrieved via find()`, () => { + const obj = { + job: { + orders: [ + { orderId: `order-1`, orderBinInt: 1 }, + { orderId: `order-2`, orderBinInt: 2 }, + ], + }, + } + const { proxy, getChanges } = createChangeProxy(obj) + + // This is the exact pattern from the user's reproduction: + // Using .find() to get an array item and then modifying it + const order = proxy.job.orders.find( + (order) => order.orderId === `order-1` + ) + if (order) { + order.orderBinInt = 99 + } + + // The changes should be tracked - this currently fails + const changes = getChanges() + expect(Object.keys(changes).length).toBeGreaterThan(0) + expect(changes.job?.orders?.[0]?.orderBinInt).toBe(99) + }) + + // Additional tests for other array iteration methods that should track changes + it(`should track changes when modifying array items via forEach`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) + + proxy.items.forEach((item) => { + item.value = item.value * 2 + }) + + const changes = getChanges() + expect(Object.keys(changes).length).toBeGreaterThan(0) + }) + + it(`should track changes when modifying array items via for...of`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) + + for (const item of proxy.items) { + item.value = item.value * 2 + } + + const changes = getChanges() + expect(Object.keys(changes).length).toBeGreaterThan(0) + }) + + it(`should track changes when modifying array items via index access`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) + + // Direct index access should work + const firstItem = proxy.items[0] + if (firstItem) { + firstItem.value = 100 + } + + const changes = getChanges() + expect(Object.keys(changes).length).toBeGreaterThan(0) + }) }) }) From da2727536a31a3c417b24e9bac64970d67bc1e17 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Nov 2025 21:30:48 +0000 Subject: [PATCH 03/13] fix: track changes to array items accessed via iteration methods This fixes a bug where modifications to array items retrieved via iteration methods like find(), forEach(), and for...of were not being tracked by the change proxy. The root cause was that array iteration methods were returning raw array elements instead of proxied versions. When users modified these raw elements, the changes were not tracked, causing getChanges() to return an empty object. This directly caused the createOptimisticAction bug where mutationFn was never called when using patterns like: draft.nested.array.find(...).property = value The fix adds handling for array iteration methods similar to how Map/Set iteration is already handled: - find(), findLast(): Return proxied element when found - filter(): Return array of proxied elements - forEach(), map(), flatMap(), some(), every(): Pass proxied elements to callbacks so mutations are tracked - reduce(), reduceRight(): Pass proxied elements to reducer callback - Symbol.iterator (for...of): Return iterator that yields proxied elements Fixes the Discord-reported bug where syncMode "on-demand" collections with createOptimisticAction would have onMutate run but mutationFn never called. --- packages/db/src/proxy.ts | 176 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index baffc1743..3ea865402 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -411,6 +411,182 @@ export function createChangeProxy< return result } } + + // For Array methods that iterate with callbacks and may return elements + // These need to pass proxied elements to callbacks and return proxied results + const callbackIterationMethods = new Set([ + `find`, + `findLast`, + `findIndex`, + `findLastIndex`, + `filter`, + `map`, + `flatMap`, + `forEach`, + `some`, + `every`, + `reduce`, + `reduceRight`, + ]) + + if (callbackIterationMethods.has(methodName)) { + return function (...args: Array) { + const callback = args[0] + if (typeof callback !== `function`) { + return value.apply(changeTracker.copy_, args) + } + + // Create a helper to get proxied version of an array element + const getProxiedElement = ( + element: unknown, + index: number + ): unknown => { + if ( + element && + typeof element === `object` && + !((element as any) instanceof Date) && + !((element as any) instanceof RegExp) && + !isTemporal(element) + ) { + // Create a parent reference for the array element + const nestedParent = { + tracker: changeTracker, + prop: String(index), + } + const { proxy: elementProxy } = memoizedCreateChangeProxy( + element as Record, + nestedParent + ) + return elementProxy + } + return element + } + + // Wrap the callback to pass proxied elements + const wrappedCallback = function ( + this: unknown, + element: unknown, + index: number, + array: unknown + ) { + const proxiedElement = getProxiedElement(element, index) + return callback.call(this, proxiedElement, index, array) + } + + // For reduce/reduceRight, the callback signature is different + if (methodName === `reduce` || methodName === `reduceRight`) { + const reduceCallback = function ( + this: unknown, + accumulator: unknown, + element: unknown, + index: number, + array: unknown + ) { + const proxiedElement = getProxiedElement(element, index) + return callback.call( + this, + accumulator, + proxiedElement, + index, + array + ) + } + return value.apply(changeTracker.copy_, [ + reduceCallback, + ...args.slice(1), + ]) + } + + const result = value.apply(changeTracker.copy_, [ + wrappedCallback, + ...args.slice(1), + ]) + + // For find/findLast, proxy the returned element if it's an object + if ( + (methodName === `find` || methodName === `findLast`) && + result && + typeof result === `object` + ) { + // Find the index of the result in the array + const foundIndex = ( + changeTracker.copy_ as Array + ).indexOf(result) + if (foundIndex !== -1) { + return getProxiedElement(result, foundIndex) + } + } + + // For filter/map/flatMap, proxy each element in the result array + if ( + (methodName === `filter` || + methodName === `map` || + methodName === `flatMap`) && + Array.isArray(result) + ) { + // For filter, the result contains original elements + // For map/flatMap, the result contains new elements from callback + // We don't need to proxy map/flatMap results as they're new objects + if (methodName === `filter`) { + return result.map((element) => { + const originalIndex = ( + changeTracker.copy_ as Array + ).indexOf(element) + if (originalIndex !== -1) { + return getProxiedElement(element, originalIndex) + } + return element + }) + } + } + + return result + } + } + + // Handle array Symbol.iterator for for...of loops + if (prop === Symbol.iterator) { + return function () { + const array = changeTracker.copy_ as Array + let index = 0 + + return { + next() { + if (index >= array.length) { + return { done: true, value: undefined } + } + + const element = array[index] + let proxiedElement = element + + // Proxy object elements + if ( + element && + typeof element === `object` && + !((element as any) instanceof Date) && + !((element as any) instanceof RegExp) && + !isTemporal(element) + ) { + const nestedParent = { + tracker: changeTracker, + prop: String(index), + } + const { proxy: elementProxy } = memoizedCreateChangeProxy( + element as Record, + nestedParent + ) + proxiedElement = elementProxy + } + + index++ + return { done: false, value: proxiedElement } + }, + [Symbol.iterator]() { + return this + }, + } + } + } } // For Map and Set methods that modify the collection From 74277a001a4582fbdce6d3eb7853284a57fd7809 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Nov 2025 23:17:09 +0000 Subject: [PATCH 04/13] changeset --- .changeset/fix-array-iteration-proxy.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 .changeset/fix-array-iteration-proxy.md diff --git a/.changeset/fix-array-iteration-proxy.md b/.changeset/fix-array-iteration-proxy.md new file mode 100644 index 000000000..c02ccb9f7 --- /dev/null +++ b/.changeset/fix-array-iteration-proxy.md @@ -0,0 +1,18 @@ +--- +"@tanstack/db": patch +--- + +Fix change tracking for array items accessed via iteration methods (find, forEach, for...of, etc.) + +Previously, modifications to array items retrieved via iteration methods were not tracked by the change proxy because these methods returned raw array elements instead of proxied versions. This caused `getChanges()` to return an empty object, which in turn caused `createOptimisticAction`'s `mutationFn` to never be called when using patterns like: + +```ts +collection.update(id, (draft) => { + const item = draft.items.find(x => x.id === targetId) + if (item) { + item.value = newValue // This change was not tracked! + } +}) +``` + +The fix adds proxy handling for array iteration methods similar to how Map/Set iteration is already handled, ensuring that callbacks receive proxied elements and returned elements are properly proxied. From 17e4bd1468bb890b9fd576d2a6197576feddb1af Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Nov 2025 23:20:22 +0000 Subject: [PATCH 05/13] test: clean up test comments and remove redundant on-demand tests - Remove "bug" references from proxy test comments - Remove on-demand syncMode tests from optimistic-action.test.ts that were always passing (the actual fix was in proxy.ts array iteration handling) --- packages/db/tests/optimistic-action.test.ts | 352 -------------------- packages/db/tests/proxy.test.ts | 9 +- 2 files changed, 2 insertions(+), 359 deletions(-) diff --git a/packages/db/tests/optimistic-action.test.ts b/packages/db/tests/optimistic-action.test.ts index 0fb286cb9..95f43c242 100644 --- a/packages/db/tests/optimistic-action.test.ts +++ b/packages/db/tests/optimistic-action.test.ts @@ -1,6 +1,5 @@ import { describe, expect, expectTypeOf, it, vi } from "vitest" import { createCollection, createOptimisticAction } from "../src" -import { createLiveQueryCollection, eq } from "../src/query" import type { MutationFnParams, Transaction, @@ -208,357 +207,6 @@ describe(`createOptimisticAction`, () => { expectTypeOf(userAction).returns.toEqualTypeOf() }) - // Test with syncMode "on-demand" - it(`should call mutationFn when using syncMode on-demand`, async () => { - // This test reproduces the bug where mutationFn is not called - // when the collection is configured with syncMode: "on-demand" - // Bug report: https://discord.com/channels/... - // - onMutate runs but mutationFn never runs - // - works with eager mode but not on-demand - - const onMutateMock = vi.fn() - const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) - - // Create a collection with syncMode: "on-demand" - // This requires a loadSubset handler - const collection = createCollection<{ id: string; text: string }>({ - id: `on-demand-collection`, - getKey: (item) => item.id, - syncMode: `on-demand`, - sync: { - sync: ({ markReady }) => { - // For on-demand mode, we mark ready immediately but don't load data - // Data is loaded on-demand via loadSubset - markReady() - - return { - loadSubset: () => { - // No-op for testing - just return true to indicate sync - return true - }, - } - }, - }, - }) - - // Create an optimistic action - const addTodo = createOptimisticAction({ - onMutate: (text) => { - collection.insert({ id: `1`, text }) - onMutateMock(text) - }, - mutationFn: async (text, params) => { - return Promise.resolve(mutationFnMock(text, params)) - }, - }) - - // Execute the optimistic action - const transaction = addTodo(`Test Todo`) - - // Verify onMutate was called - expect(onMutateMock).toHaveBeenCalledWith(`Test Todo`) - - // Verify the optimistic update was applied to the collection - expect(collection.get(`1`)).toEqual({ id: `1`, text: `Test Todo` }) - - // Wait for the mutation to complete - await transaction.isPersisted.promise - - // BUG: mutationFn should be called but it's not! - expect(mutationFnMock).toHaveBeenCalledTimes(1) - }) - - // Test with syncMode "on-demand" where collection has NOT started sync yet - it(`should call mutationFn when collection is not started (idle)`, async () => { - // This test checks if mutationFn is called when the collection hasn't started sync - const onMutateMock = vi.fn() - const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) - - // Create a collection that doesn't start sync automatically - const collection = createCollection<{ id: string; text: string }>({ - id: `idle-collection`, - getKey: (item) => item.id, - startSync: false, - sync: { - sync: () => { - // No-op sync for testing - }, - }, - }) - - // Create an optimistic action - const addTodo = createOptimisticAction({ - onMutate: (text) => { - collection.insert({ id: `1`, text }) - onMutateMock(text) - }, - mutationFn: async (text, params) => { - return Promise.resolve(mutationFnMock(text, params)) - }, - }) - - // Execute the optimistic action (collection is in idle state) - const transaction = addTodo(`Test Todo`) - - // Verify onMutate was called - expect(onMutateMock).toHaveBeenCalledWith(`Test Todo`) - - // Verify the optimistic update was applied - expect(collection.get(`1`)).toEqual({ id: `1`, text: `Test Todo` }) - - // Wait for the mutation to complete - await transaction.isPersisted.promise - - // mutationFn should be called - expect(mutationFnMock).toHaveBeenCalledTimes(1) - }) - - // Test with syncMode "on-demand" where sync is in loading state (not ready yet) - it(`should call mutationFn when collection is loading (not ready)`, async () => { - // This test checks if mutationFn is called when the collection is still loading - const onMutateMock = vi.fn() - const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) - - // Create a collection that starts sync but doesn't call markReady - const collection = createCollection<{ id: string; text: string }>({ - id: `loading-collection`, - getKey: (item) => item.id, - startSync: true, - sync: { - sync: () => { - // Intentionally don't call markReady - collection stays in "loading" state - return () => {} - }, - }, - }) - - // Create an optimistic action - const addTodo = createOptimisticAction({ - onMutate: (text) => { - collection.insert({ id: `1`, text }) - onMutateMock(text) - }, - mutationFn: async (text, params) => { - return Promise.resolve(mutationFnMock(text, params)) - }, - }) - - // Execute the optimistic action (collection is in loading state) - const transaction = addTodo(`Test Todo`) - - // Verify onMutate was called - expect(onMutateMock).toHaveBeenCalledWith(`Test Todo`) - - // Verify the optimistic update was applied - expect(collection.get(`1`)).toEqual({ id: `1`, text: `Test Todo` }) - - // Wait for the mutation to complete - await transaction.isPersisted.promise - - // mutationFn should be called - expect(mutationFnMock).toHaveBeenCalledTimes(1) - }) - - // Test with on-demand collection and a live query with filters - the reported scenario - it(`should call mutationFn with on-demand collection and live query filter`, async () => { - // This test attempts to reproduce the exact bug scenario: - // - Base collection with syncMode: "on-demand" - // - Live query collection with filters on top - // - createOptimisticAction used to mutate the base collection - // - Bug: onMutate runs but mutationFn never runs - - const onMutateMock = vi.fn() - const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) - - // Create a base collection with syncMode: "on-demand" - type Todo = { id: string; text: string; status: string } - const baseCollection = createCollection({ - id: `on-demand-base-collection`, - getKey: (item) => item.id, - syncMode: `on-demand`, - startSync: true, - sync: { - sync: ({ markReady, begin, write, commit }) => { - // Simulate on-demand mode: mark ready immediately, load data on demand - markReady() - - // Pre-populate with some data - begin() - write({ - type: `insert`, - value: { id: `1`, text: `Existing todo`, status: `active` }, - }) - commit() - - return { - loadSubset: () => { - return true - }, - } - }, - }, - }) - - // Create a live query collection with a filter on status - const activeTodos = createLiveQueryCollection({ - id: `active-todos-query`, - startSync: true, - query: (q) => - q - .from({ todo: baseCollection }) - .where(({ todo }) => eq(todo.status, `active`)) - .select(({ todo }) => ({ todo })), - }) - - // Wait for the live query to be ready - await activeTodos.preload() - - // Verify initial state - expect([...activeTodos.values()].length).toBe(1) - expect([...activeTodos.values()][0]?.todo.text).toBe(`Existing todo`) - - // Create an optimistic action to INSERT a new todo - const addTodo = createOptimisticAction<{ text: string }>({ - onMutate: (input) => { - baseCollection.insert({ id: `2`, text: input.text, status: `active` }) - onMutateMock(input) - }, - mutationFn: async (input, params) => { - return Promise.resolve(mutationFnMock(input, params)) - }, - }) - - // Execute the optimistic action - const transaction = addTodo({ text: `New todo` }) - - // Verify onMutate was called - expect(onMutateMock).toHaveBeenCalledWith({ text: `New todo` }) - - // Verify the optimistic update was applied to the base collection - expect(baseCollection.get(`2`)).toEqual({ - id: `2`, - text: `New todo`, - status: `active`, - }) - - // Wait for the mutation to complete - await transaction.isPersisted.promise - - // BUG: mutationFn should be called! - expect(mutationFnMock).toHaveBeenCalledTimes(1) - }) - - // Test UPDATE scenario which might have different behavior - it(`should call mutationFn when UPDATE is performed on on-demand collection`, async () => { - const onMutateMock = vi.fn() - const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) - - type Todo = { id: string; text: string; status: string } - const collection = createCollection({ - id: `on-demand-update-collection`, - getKey: (item) => item.id, - syncMode: `on-demand`, - startSync: true, - sync: { - sync: ({ markReady, begin, write, commit }) => { - markReady() - - // Pre-populate with data - begin() - write({ - type: `insert`, - value: { id: `1`, text: `Original text`, status: `active` }, - }) - commit() - - return { - loadSubset: () => true, - } - }, - }, - }) - - // Create an optimistic action to UPDATE an existing todo - const updateTodo = createOptimisticAction<{ id: string; text: string }>({ - onMutate: (input) => { - collection.update(input.id, (draft) => { - draft.text = input.text - }) - onMutateMock(input) - }, - mutationFn: async (input, params) => { - return Promise.resolve(mutationFnMock(input, params)) - }, - }) - - // Execute the optimistic action - const transaction = updateTodo({ id: `1`, text: `Updated text` }) - - // Verify onMutate was called - expect(onMutateMock).toHaveBeenCalledWith({ id: `1`, text: `Updated text` }) - - // Verify the optimistic update was applied - expect(collection.get(`1`)?.text).toBe(`Updated text`) - - // Wait for the mutation to complete - await transaction.isPersisted.promise - - // mutationFn should be called - expect(mutationFnMock).toHaveBeenCalledTimes(1) - }) - - // Debug test: verify mutations array is populated correctly - it(`should have mutations in the transaction after onMutate completes`, async () => { - const onMutateMock = vi.fn() - const mutationFnMock = vi.fn().mockResolvedValue({ success: true }) - - // Create an on-demand collection with live query filter - type Todo = { id: string; text: string; status: string } - const baseCollection = createCollection({ - id: `debug-collection`, - getKey: (item) => item.id, - syncMode: `on-demand`, - startSync: true, - sync: { - sync: ({ markReady }) => { - markReady() - return { - loadSubset: () => true, - } - }, - }, - }) - - // Track mutations count at mutationFn call time - let mutationsAtMutationFn: number | undefined - - const addTodo = createOptimisticAction<{ text: string }>({ - onMutate: (input) => { - baseCollection.insert({ id: `1`, text: input.text, status: `active` }) - onMutateMock(input) - }, - mutationFn: async (input, params) => { - // Record the number of mutations at this point - mutationsAtMutationFn = params.transaction.mutations.length - return Promise.resolve(mutationFnMock(input, params)) - }, - }) - - const transaction = addTodo({ text: `Test` }) - - // Verify onMutate was called - expect(onMutateMock).toHaveBeenCalled() - - // Wait for the transaction to complete - await transaction.isPersisted.promise - - // Verify mutationFn was called - expect(mutationFnMock).toHaveBeenCalledTimes(1) - - // Verify there was at least one mutation - expect(mutationsAtMutationFn).toBeGreaterThan(0) - }) - // Test error handling it(`should handle errors in mutationFn correctly`, async () => { // Setup a mock collection diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index 33bf2cd05..802d60ef8 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -1654,9 +1654,7 @@ describe(`Proxy Library`, () => { expect(obj.date).toEqual(originalDate) }) - // BUG: Array.find() returns unproxied items, so changes to them aren't tracked - // This is the root cause of the createOptimisticAction bug where mutationFn - // is never called when modifying nested array items via .find() + // Test that array iteration methods return proxied elements for change tracking it(`should track changes when modifying array items retrieved via find()`, () => { const obj = { job: { @@ -1668,8 +1666,7 @@ describe(`Proxy Library`, () => { } const { proxy, getChanges } = createChangeProxy(obj) - // This is the exact pattern from the user's reproduction: - // Using .find() to get an array item and then modifying it + // Use find() to get an array item and modify it const order = proxy.job.orders.find( (order) => order.orderId === `order-1` ) @@ -1677,13 +1674,11 @@ describe(`Proxy Library`, () => { order.orderBinInt = 99 } - // The changes should be tracked - this currently fails const changes = getChanges() expect(Object.keys(changes).length).toBeGreaterThan(0) expect(changes.job?.orders?.[0]?.orderBinInt).toBe(99) }) - // Additional tests for other array iteration methods that should track changes it(`should track changes when modifying array items via forEach`, () => { const obj = { items: [ From 3ec0264180985b21f69b1471786a8779d244921e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Nov 2025 01:17:29 +0000 Subject: [PATCH 06/13] fix: correct TypeScript type casts for array operations in proxy Cast through 'unknown' first when converting changeTracker.copy_ to Array to satisfy TypeScript's type checking. --- packages/db/src/proxy.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index 3ea865402..60bff8cd6 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -510,7 +510,7 @@ export function createChangeProxy< ) { // Find the index of the result in the array const foundIndex = ( - changeTracker.copy_ as Array + changeTracker.copy_ as unknown as Array ).indexOf(result) if (foundIndex !== -1) { return getProxiedElement(result, foundIndex) @@ -530,7 +530,7 @@ export function createChangeProxy< if (methodName === `filter`) { return result.map((element) => { const originalIndex = ( - changeTracker.copy_ as Array + changeTracker.copy_ as unknown as Array ).indexOf(element) if (originalIndex !== -1) { return getProxiedElement(element, originalIndex) @@ -547,7 +547,7 @@ export function createChangeProxy< // Handle array Symbol.iterator for for...of loops if (prop === Symbol.iterator) { return function () { - const array = changeTracker.copy_ as Array + const array = changeTracker.copy_ as unknown as Array let index = 0 return { From 3a785e5d752f8ee4231a9b570aadcc0cac99a028 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Nov 2025 03:23:03 +0000 Subject: [PATCH 07/13] style: format changeset file with prettier --- .changeset/fix-array-iteration-proxy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/fix-array-iteration-proxy.md b/.changeset/fix-array-iteration-proxy.md index c02ccb9f7..449cb0935 100644 --- a/.changeset/fix-array-iteration-proxy.md +++ b/.changeset/fix-array-iteration-proxy.md @@ -8,7 +8,7 @@ Previously, modifications to array items retrieved via iteration methods were no ```ts collection.update(id, (draft) => { - const item = draft.items.find(x => x.id === targetId) + const item = draft.items.find((x) => x.id === targetId) if (item) { item.value = newValue // This change was not tracked! } From 70b46ceffab304d8d8058a5e124c066ffb0a1a29 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Nov 2025 03:31:05 +0000 Subject: [PATCH 08/13] refactor: improve proxy code quality and add tests - Hoist CALLBACK_ITERATION_METHODS Set to module scope for better perf - Extract isProxiableObject helper to reduce code duplication - Add tests for filter(), findLast(), some(), and reduce() change tracking --- packages/db/src/proxy.ts | 79 +++++++++++++++++---------------- packages/db/tests/proxy.test.ts | 79 +++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 39 deletions(-) diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index 60bff8cd6..e0c45069e 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -5,6 +5,40 @@ import { deepEquals, isTemporal } from "./utils" +/** + * Set of array methods that iterate with callbacks and may return elements. + * Hoisted to module scope to avoid creating a new Set on every property access. + */ +const CALLBACK_ITERATION_METHODS = new Set([ + `find`, + `findLast`, + `findIndex`, + `findLastIndex`, + `filter`, + `map`, + `flatMap`, + `forEach`, + `some`, + `every`, + `reduce`, + `reduceRight`, +]) + +/** + * Check if a value is a proxiable object (not Date, RegExp, or Temporal) + */ +function isProxiableObject( + value: unknown +): value is Record { + return ( + value !== null && + typeof value === `object` && + !((value as any) instanceof Date) && + !((value as any) instanceof RegExp) && + !isTemporal(value) + ) +} + /** * Simple debug utility that only logs when debug mode is enabled * Set DEBUG to true in localStorage to enable debug logging @@ -414,22 +448,7 @@ export function createChangeProxy< // For Array methods that iterate with callbacks and may return elements // These need to pass proxied elements to callbacks and return proxied results - const callbackIterationMethods = new Set([ - `find`, - `findLast`, - `findIndex`, - `findLastIndex`, - `filter`, - `map`, - `flatMap`, - `forEach`, - `some`, - `every`, - `reduce`, - `reduceRight`, - ]) - - if (callbackIterationMethods.has(methodName)) { + if (CALLBACK_ITERATION_METHODS.has(methodName)) { return function (...args: Array) { const callback = args[0] if (typeof callback !== `function`) { @@ -441,20 +460,14 @@ export function createChangeProxy< element: unknown, index: number ): unknown => { - if ( - element && - typeof element === `object` && - !((element as any) instanceof Date) && - !((element as any) instanceof RegExp) && - !isTemporal(element) - ) { + if (isProxiableObject(element)) { // Create a parent reference for the array element const nestedParent = { tracker: changeTracker, prop: String(index), } const { proxy: elementProxy } = memoizedCreateChangeProxy( - element as Record, + element, nestedParent ) return elementProxy @@ -560,19 +573,13 @@ export function createChangeProxy< let proxiedElement = element // Proxy object elements - if ( - element && - typeof element === `object` && - !((element as any) instanceof Date) && - !((element as any) instanceof RegExp) && - !isTemporal(element) - ) { + if (isProxiableObject(element)) { const nestedParent = { tracker: changeTracker, prop: String(index), } const { proxy: elementProxy } = memoizedCreateChangeProxy( - element as Record, + element, nestedParent ) proxiedElement = elementProxy @@ -826,13 +833,7 @@ export function createChangeProxy< } // If the value is an object (but not Date, RegExp, or Temporal), create a proxy for it - if ( - value && - typeof value === `object` && - !((value as any) instanceof Date) && - !((value as any) instanceof RegExp) && - !isTemporal(value) - ) { + if (isProxiableObject(value)) { // Create a parent reference for the nested object const nestedParent = { tracker: changeTracker, diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index 802d60ef8..720b15ec5 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -1731,5 +1731,84 @@ describe(`Proxy Library`, () => { const changes = getChanges() expect(Object.keys(changes).length).toBeGreaterThan(0) }) + + it(`should track changes when modifying items from filter() result`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) + + const filtered = proxy.items.filter((item) => item.id === 1) + const first = filtered[0] + if (first) { + first.value = 42 + } + + const changes = getChanges() + expect(changes.items?.[0]?.value).toBe(42) + }) + + it(`should track changes when modifying array items retrieved via findLast()`, () => { + const obj = { + job: { + orders: [ + { orderId: `order-1`, orderBinInt: 1 }, + { orderId: `order-2`, orderBinInt: 2 }, + ], + }, + } + const { proxy, getChanges } = createChangeProxy(obj) + + const order = proxy.job.orders.findLast((order) => + order.orderId.startsWith(`order-`) + ) + if (order) { + order.orderBinInt = 123 + } + + const changes = getChanges() + expect(changes.job?.orders?.[1]?.orderBinInt).toBe(123) + }) + + it(`should track changes when modifying array items inside some() callback`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) + + proxy.items.some((item) => { + item.value = item.value * 2 + return false + }) + + const changes = getChanges() + expect(changes.items?.[0]?.value).toBe(20) + expect(changes.items?.[1]?.value).toBe(40) + }) + + it(`should track changes when modifying array items inside reduce() callback`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) + + proxy.items.reduce((acc, item) => { + item.value = item.value + 1 + return acc + item.value + }, 0) + + const changes = getChanges() + expect(changes.items?.[0]?.value).toBe(11) + expect(changes.items?.[1]?.value).toBe(21) + }) }) }) From 752fa93bd9e8842e95735bb77c6e7fe17c46e58a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Nov 2025 03:34:58 +0000 Subject: [PATCH 09/13] fix: add type assertion for findLast test (ES2023 method) --- packages/db/tests/proxy.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index 720b15ec5..f3293fd95 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -1762,9 +1762,12 @@ describe(`Proxy Library`, () => { } const { proxy, getChanges } = createChangeProxy(obj) - const order = proxy.job.orders.findLast((order) => - order.orderId.startsWith(`order-`) - ) + // Use type assertion to call findLast (ES2023 method) + type Order = { orderId: string; orderBinInt: number } + const orders = proxy.job.orders as unknown as { + findLast: (predicate: (o: Order) => boolean) => Order | undefined + } + const order = orders.findLast((o) => o.orderId.startsWith(`order-`)) if (order) { order.orderBinInt = 123 } From d369b3cdfbcb25ce984af9b780074998ce9e3a9e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Nov 2025 15:40:57 +0000 Subject: [PATCH 10/13] refactor: extract array iteration handlers and group tests - Extract createArrayIterationHandler() for find/filter/forEach/etc. - Extract createArrayIteratorHandler() for Symbol.iterator - Reduces main get() handler by ~120 lines - Group array iteration tests in describe block per review feedback --- packages/db/src/proxy.ts | 315 +++++++++++++++++--------------- packages/db/tests/proxy.test.ts | 269 +++++++++++++-------------- 2 files changed, 307 insertions(+), 277 deletions(-) diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index e0c45069e..be1b8dd0a 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -39,6 +39,165 @@ function isProxiableObject( ) } +/** + * Creates a handler for array iteration methods that ensures proxied elements + * are passed to callbacks and returned from methods like find/filter. + */ +function createArrayIterationHandler( + methodName: string, + methodFn: (...args: Array) => unknown, + changeTracker: ChangeTracker, + memoizedCreateChangeProxy: ( + obj: Record, + parent?: { + tracker: ChangeTracker> + prop: string | symbol + } + ) => { proxy: Record } +): ((...args: Array) => unknown) | undefined { + if (!CALLBACK_ITERATION_METHODS.has(methodName)) { + return undefined + } + + return function (...args: Array) { + const callback = args[0] + if (typeof callback !== `function`) { + return methodFn.apply(changeTracker.copy_, args) + } + + // Create a helper to get proxied version of an array element + const getProxiedElement = (element: unknown, index: number): unknown => { + if (isProxiableObject(element)) { + const nestedParent = { + tracker: changeTracker as unknown as ChangeTracker< + Record + >, + prop: String(index), + } + const { proxy: elementProxy } = memoizedCreateChangeProxy( + element, + nestedParent + ) + return elementProxy + } + return element + } + + // Wrap the callback to pass proxied elements + const wrappedCallback = function ( + this: unknown, + element: unknown, + index: number, + array: unknown + ) { + const proxiedElement = getProxiedElement(element, index) + return callback.call(this, proxiedElement, index, array) + } + + // For reduce/reduceRight, the callback signature is different + if (methodName === `reduce` || methodName === `reduceRight`) { + const reduceCallback = function ( + this: unknown, + accumulator: unknown, + element: unknown, + index: number, + array: unknown + ) { + const proxiedElement = getProxiedElement(element, index) + return callback.call(this, accumulator, proxiedElement, index, array) + } + return methodFn.apply(changeTracker.copy_, [ + reduceCallback, + ...args.slice(1), + ]) + } + + const result = methodFn.apply(changeTracker.copy_, [ + wrappedCallback, + ...args.slice(1), + ]) + + // For find/findLast, proxy the returned element if it's an object + if ( + (methodName === `find` || methodName === `findLast`) && + result && + typeof result === `object` + ) { + const foundIndex = ( + changeTracker.copy_ as unknown as Array + ).indexOf(result) + if (foundIndex !== -1) { + return getProxiedElement(result, foundIndex) + } + } + + // For filter, proxy each element in the result array + if (methodName === `filter` && Array.isArray(result)) { + return result.map((element) => { + const originalIndex = ( + changeTracker.copy_ as unknown as Array + ).indexOf(element) + if (originalIndex !== -1) { + return getProxiedElement(element, originalIndex) + } + return element + }) + } + + return result + } +} + +/** + * Creates a Symbol.iterator handler for arrays that yields proxied elements. + */ +function createArrayIteratorHandler( + changeTracker: ChangeTracker, + memoizedCreateChangeProxy: ( + obj: Record, + parent?: { + tracker: ChangeTracker> + prop: string | symbol + } + ) => { proxy: Record } +): () => Iterator { + return function () { + const array = changeTracker.copy_ as unknown as Array + let index = 0 + + return { + next() { + if (index >= array.length) { + return { done: true, value: undefined } + } + + const element = array[index] + let proxiedElement = element + + if (isProxiableObject(element)) { + const nestedParent = { + tracker: changeTracker as unknown as ChangeTracker< + Record + >, + prop: String(index), + } + const { proxy: elementProxy } = memoizedCreateChangeProxy( + element, + nestedParent + ) + proxiedElement = elementProxy + } + + index++ + return { done: false, value: proxiedElement } + }, + [Symbol.iterator]() { + return this + }, + } + } +} + /** * Simple debug utility that only logs when debug mode is enabled * Set DEBUG to true in localStorage to enable debug logging @@ -446,153 +605,23 @@ export function createChangeProxy< } } - // For Array methods that iterate with callbacks and may return elements - // These need to pass proxied elements to callbacks and return proxied results - if (CALLBACK_ITERATION_METHODS.has(methodName)) { - return function (...args: Array) { - const callback = args[0] - if (typeof callback !== `function`) { - return value.apply(changeTracker.copy_, args) - } - - // Create a helper to get proxied version of an array element - const getProxiedElement = ( - element: unknown, - index: number - ): unknown => { - if (isProxiableObject(element)) { - // Create a parent reference for the array element - const nestedParent = { - tracker: changeTracker, - prop: String(index), - } - const { proxy: elementProxy } = memoizedCreateChangeProxy( - element, - nestedParent - ) - return elementProxy - } - return element - } - - // Wrap the callback to pass proxied elements - const wrappedCallback = function ( - this: unknown, - element: unknown, - index: number, - array: unknown - ) { - const proxiedElement = getProxiedElement(element, index) - return callback.call(this, proxiedElement, index, array) - } - - // For reduce/reduceRight, the callback signature is different - if (methodName === `reduce` || methodName === `reduceRight`) { - const reduceCallback = function ( - this: unknown, - accumulator: unknown, - element: unknown, - index: number, - array: unknown - ) { - const proxiedElement = getProxiedElement(element, index) - return callback.call( - this, - accumulator, - proxiedElement, - index, - array - ) - } - return value.apply(changeTracker.copy_, [ - reduceCallback, - ...args.slice(1), - ]) - } - - const result = value.apply(changeTracker.copy_, [ - wrappedCallback, - ...args.slice(1), - ]) - - // For find/findLast, proxy the returned element if it's an object - if ( - (methodName === `find` || methodName === `findLast`) && - result && - typeof result === `object` - ) { - // Find the index of the result in the array - const foundIndex = ( - changeTracker.copy_ as unknown as Array - ).indexOf(result) - if (foundIndex !== -1) { - return getProxiedElement(result, foundIndex) - } - } - - // For filter/map/flatMap, proxy each element in the result array - if ( - (methodName === `filter` || - methodName === `map` || - methodName === `flatMap`) && - Array.isArray(result) - ) { - // For filter, the result contains original elements - // For map/flatMap, the result contains new elements from callback - // We don't need to proxy map/flatMap results as they're new objects - if (methodName === `filter`) { - return result.map((element) => { - const originalIndex = ( - changeTracker.copy_ as unknown as Array - ).indexOf(element) - if (originalIndex !== -1) { - return getProxiedElement(element, originalIndex) - } - return element - }) - } - } - - return result - } + // Handle array iteration methods (find, filter, forEach, etc.) + const iterationHandler = createArrayIterationHandler( + methodName, + value, + changeTracker, + memoizedCreateChangeProxy + ) + if (iterationHandler) { + return iterationHandler } // Handle array Symbol.iterator for for...of loops if (prop === Symbol.iterator) { - return function () { - const array = changeTracker.copy_ as unknown as Array - let index = 0 - - return { - next() { - if (index >= array.length) { - return { done: true, value: undefined } - } - - const element = array[index] - let proxiedElement = element - - // Proxy object elements - if (isProxiableObject(element)) { - const nestedParent = { - tracker: changeTracker, - prop: String(index), - } - const { proxy: elementProxy } = memoizedCreateChangeProxy( - element, - nestedParent - ) - proxiedElement = elementProxy - } - - index++ - return { done: false, value: proxiedElement } - }, - [Symbol.iterator]() { - return this - }, - } - } + return createArrayIteratorHandler( + changeTracker, + memoizedCreateChangeProxy + ) } } diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index f3293fd95..41bb0fee5 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -1654,164 +1654,165 @@ describe(`Proxy Library`, () => { expect(obj.date).toEqual(originalDate) }) - // Test that array iteration methods return proxied elements for change tracking - it(`should track changes when modifying array items retrieved via find()`, () => { - const obj = { - job: { - orders: [ - { orderId: `order-1`, orderBinInt: 1 }, - { orderId: `order-2`, orderBinInt: 2 }, - ], - }, - } - const { proxy, getChanges } = createChangeProxy(obj) + describe(`array iteration methods`, () => { + it(`should track changes when modifying array items retrieved via find()`, () => { + const obj = { + job: { + orders: [ + { orderId: `order-1`, orderBinInt: 1 }, + { orderId: `order-2`, orderBinInt: 2 }, + ], + }, + } + const { proxy, getChanges } = createChangeProxy(obj) - // Use find() to get an array item and modify it - const order = proxy.job.orders.find( - (order) => order.orderId === `order-1` - ) - if (order) { - order.orderBinInt = 99 - } + // Use find() to get an array item and modify it + const order = proxy.job.orders.find( + (order) => order.orderId === `order-1` + ) + if (order) { + order.orderBinInt = 99 + } - const changes = getChanges() - expect(Object.keys(changes).length).toBeGreaterThan(0) - expect(changes.job?.orders?.[0]?.orderBinInt).toBe(99) - }) + const changes = getChanges() + expect(Object.keys(changes).length).toBeGreaterThan(0) + expect(changes.job?.orders?.[0]?.orderBinInt).toBe(99) + }) - it(`should track changes when modifying array items via forEach`, () => { - const obj = { - items: [ - { id: 1, value: 10 }, - { id: 2, value: 20 }, - ], - } - const { proxy, getChanges } = createChangeProxy(obj) + it(`should track changes when modifying array items via forEach`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) - proxy.items.forEach((item) => { - item.value = item.value * 2 + proxy.items.forEach((item) => { + item.value = item.value * 2 + }) + + const changes = getChanges() + expect(Object.keys(changes).length).toBeGreaterThan(0) }) - const changes = getChanges() - expect(Object.keys(changes).length).toBeGreaterThan(0) - }) + it(`should track changes when modifying array items via for...of`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) - it(`should track changes when modifying array items via for...of`, () => { - const obj = { - items: [ - { id: 1, value: 10 }, - { id: 2, value: 20 }, - ], - } - const { proxy, getChanges } = createChangeProxy(obj) + for (const item of proxy.items) { + item.value = item.value * 2 + } - for (const item of proxy.items) { - item.value = item.value * 2 - } + const changes = getChanges() + expect(Object.keys(changes).length).toBeGreaterThan(0) + }) - const changes = getChanges() - expect(Object.keys(changes).length).toBeGreaterThan(0) - }) + it(`should track changes when modifying array items via index access`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) - it(`should track changes when modifying array items via index access`, () => { - const obj = { - items: [ - { id: 1, value: 10 }, - { id: 2, value: 20 }, - ], - } - const { proxy, getChanges } = createChangeProxy(obj) + // Direct index access should work + const firstItem = proxy.items[0] + if (firstItem) { + firstItem.value = 100 + } - // Direct index access should work - const firstItem = proxy.items[0] - if (firstItem) { - firstItem.value = 100 - } + const changes = getChanges() + expect(Object.keys(changes).length).toBeGreaterThan(0) + }) - const changes = getChanges() - expect(Object.keys(changes).length).toBeGreaterThan(0) - }) + it(`should track changes when modifying items from filter() result`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) - it(`should track changes when modifying items from filter() result`, () => { - const obj = { - items: [ - { id: 1, value: 10 }, - { id: 2, value: 20 }, - ], - } - const { proxy, getChanges } = createChangeProxy(obj) + const filtered = proxy.items.filter((item) => item.id === 1) + const first = filtered[0] + if (first) { + first.value = 42 + } - const filtered = proxy.items.filter((item) => item.id === 1) - const first = filtered[0] - if (first) { - first.value = 42 - } + const changes = getChanges() + expect(changes.items?.[0]?.value).toBe(42) + }) - const changes = getChanges() - expect(changes.items?.[0]?.value).toBe(42) - }) + it(`should track changes when modifying array items retrieved via findLast()`, () => { + const obj = { + job: { + orders: [ + { orderId: `order-1`, orderBinInt: 1 }, + { orderId: `order-2`, orderBinInt: 2 }, + ], + }, + } + const { proxy, getChanges } = createChangeProxy(obj) - it(`should track changes when modifying array items retrieved via findLast()`, () => { - const obj = { - job: { - orders: [ - { orderId: `order-1`, orderBinInt: 1 }, - { orderId: `order-2`, orderBinInt: 2 }, - ], - }, - } - const { proxy, getChanges } = createChangeProxy(obj) + // Use type assertion to call findLast (ES2023 method) + type Order = { orderId: string; orderBinInt: number } + const orders = proxy.job.orders as unknown as { + findLast: (predicate: (o: Order) => boolean) => Order | undefined + } + const order = orders.findLast((o) => o.orderId.startsWith(`order-`)) + if (order) { + order.orderBinInt = 123 + } - // Use type assertion to call findLast (ES2023 method) - type Order = { orderId: string; orderBinInt: number } - const orders = proxy.job.orders as unknown as { - findLast: (predicate: (o: Order) => boolean) => Order | undefined - } - const order = orders.findLast((o) => o.orderId.startsWith(`order-`)) - if (order) { - order.orderBinInt = 123 - } + const changes = getChanges() + expect(changes.job?.orders?.[1]?.orderBinInt).toBe(123) + }) - const changes = getChanges() - expect(changes.job?.orders?.[1]?.orderBinInt).toBe(123) - }) + it(`should track changes when modifying array items inside some() callback`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) - it(`should track changes when modifying array items inside some() callback`, () => { - const obj = { - items: [ - { id: 1, value: 10 }, - { id: 2, value: 20 }, - ], - } - const { proxy, getChanges } = createChangeProxy(obj) + proxy.items.some((item) => { + item.value = item.value * 2 + return false + }) - proxy.items.some((item) => { - item.value = item.value * 2 - return false + const changes = getChanges() + expect(changes.items?.[0]?.value).toBe(20) + expect(changes.items?.[1]?.value).toBe(40) }) - const changes = getChanges() - expect(changes.items?.[0]?.value).toBe(20) - expect(changes.items?.[1]?.value).toBe(40) - }) - - it(`should track changes when modifying array items inside reduce() callback`, () => { - const obj = { - items: [ - { id: 1, value: 10 }, - { id: 2, value: 20 }, - ], - } - const { proxy, getChanges } = createChangeProxy(obj) + it(`should track changes when modifying array items inside reduce() callback`, () => { + const obj = { + items: [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + ], + } + const { proxy, getChanges } = createChangeProxy(obj) - proxy.items.reduce((acc, item) => { - item.value = item.value + 1 - return acc + item.value - }, 0) + proxy.items.reduce((acc, item) => { + item.value = item.value + 1 + return acc + item.value + }, 0) - const changes = getChanges() - expect(changes.items?.[0]?.value).toBe(11) - expect(changes.items?.[1]?.value).toBe(21) + const changes = getChanges() + expect(changes.items?.[0]?.value).toBe(11) + expect(changes.items?.[1]?.value).toBe(21) + }) }) }) }) From 3e4b4cd79f5716fb5f85ec93093b6872be4fa653 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Nov 2025 15:55:47 +0000 Subject: [PATCH 11/13] refactor: extract more helpers and hoist constants in proxy.ts - Hoist ARRAY_MODIFYING_METHODS Set to module scope - Hoist MAP_SET_MODIFYING_METHODS Set to module scope - Hoist MAP_SET_ITERATOR_METHODS Set to module scope - Extract createModifyingMethodHandler() for unified collection mutation wrapping - Extract createMapSetIteratorHandler() for Map/Set iteration (~200 lines) - Main get() handler reduced from ~450 lines to ~100 lines --- packages/db/src/proxy.ts | 502 ++++++++++++++++++++------------------- 1 file changed, 259 insertions(+), 243 deletions(-) diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index be1b8dd0a..b86de37c9 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -24,6 +24,36 @@ const CALLBACK_ITERATION_METHODS = new Set([ `reduceRight`, ]) +/** + * Set of array methods that modify the array in place. + */ +const ARRAY_MODIFYING_METHODS = new Set([ + `pop`, + `push`, + `shift`, + `unshift`, + `splice`, + `sort`, + `reverse`, + `fill`, + `copyWithin`, +]) + +/** + * Set of Map/Set methods that modify the collection in place. + */ +const MAP_SET_MODIFYING_METHODS = new Set([`set`, `delete`, `clear`, `add`]) + +/** + * Set of Map/Set iterator methods. + */ +const MAP_SET_ITERATOR_METHODS = new Set([ + `entries`, + `keys`, + `values`, + `forEach`, +]) + /** * Check if a value is a proxiable object (not Date, RegExp, or Temporal) */ @@ -198,6 +228,210 @@ function createArrayIteratorHandler( } } +/** + * Creates a wrapper for methods that modify a collection (array, Map, Set). + * The wrapper calls the method and marks the change tracker as modified. + */ +function createModifyingMethodHandler( + methodFn: (...args: Array) => unknown, + changeTracker: ChangeTracker, + markChanged: (tracker: ChangeTracker) => void +): (...args: Array) => unknown { + return function (...args: Array) { + const result = methodFn.apply(changeTracker.copy_, args) + markChanged(changeTracker) + return result + } +} + +/** + * Creates handlers for Map/Set iterator methods (entries, keys, values, forEach). + * Returns proxied values for iteration to enable change tracking. + */ +function createMapSetIteratorHandler( + methodName: string, + prop: string | symbol, + methodFn: (...args: Array) => unknown, + target: Map | Set, + changeTracker: ChangeTracker, + memoizedCreateChangeProxy: ( + obj: Record, + parent?: Record + ) => { proxy: Record }, + markChanged: (tracker: ChangeTracker) => void +): ((...args: Array) => unknown) | undefined { + const isIteratorMethod = + MAP_SET_ITERATOR_METHODS.has(methodName) || prop === Symbol.iterator + + if (!isIteratorMethod) { + return undefined + } + + return function (this: unknown, ...args: Array) { + const result = methodFn.apply(changeTracker.copy_, args) + + // For forEach, wrap the callback to track changes + if (methodName === `forEach`) { + const callback = args[0] + if (typeof callback === `function`) { + const wrappedCallback = function ( + this: unknown, + value: unknown, + key: unknown, + collection: unknown + ) { + const cbresult = callback.call(this, value, key, collection) + markChanged(changeTracker) + return cbresult + } + return methodFn.apply(target, [wrappedCallback, ...args.slice(1)]) + } + } + + // For iterators (entries, keys, values, Symbol.iterator) + const isValueIterator = + methodName === `entries` || + methodName === `values` || + methodName === Symbol.iterator.toString() || + prop === Symbol.iterator + + if (isValueIterator) { + const originalIterator = result as Iterator + + // For values() iterator on Maps, create a value-to-key mapping + const valueToKeyMap = new Map() + if (methodName === `values` && target instanceof Map) { + for (const [key, mapValue] of ( + changeTracker.copy_ as unknown as Map + ).entries()) { + valueToKeyMap.set(mapValue, key) + } + } + + // For Set iterators, create an original-to-modified mapping + const originalToModifiedMap = new Map() + if (target instanceof Set) { + for (const setValue of ( + changeTracker.copy_ as unknown as Set + ).values()) { + originalToModifiedMap.set(setValue, setValue) + } + } + + // Return a wrapped iterator that proxies values + return { + next() { + const nextResult = originalIterator.next() + + if ( + !nextResult.done && + nextResult.value && + typeof nextResult.value === `object` + ) { + // For entries, the value is a [key, value] pair + if ( + methodName === `entries` && + Array.isArray(nextResult.value) && + nextResult.value.length === 2 + ) { + if ( + nextResult.value[1] && + typeof nextResult.value[1] === `object` + ) { + const mapKey = nextResult.value[0] + const mapParent = { + tracker: changeTracker, + prop: mapKey, + updateMap: (newValue: unknown) => { + if (changeTracker.copy_ instanceof Map) { + ;(changeTracker.copy_ as Map).set( + mapKey, + newValue + ) + } + }, + } + const { proxy: valueProxy } = memoizedCreateChangeProxy( + nextResult.value[1] as Record, + mapParent + ) + nextResult.value[1] = valueProxy + } + } else if ( + methodName === `values` || + methodName === Symbol.iterator.toString() || + prop === Symbol.iterator + ) { + // For Map values(), use the key mapping + if (methodName === `values` && target instanceof Map) { + const mapKey = valueToKeyMap.get(nextResult.value) + if (mapKey !== undefined) { + const mapParent = { + tracker: changeTracker, + prop: mapKey, + updateMap: (newValue: unknown) => { + if (changeTracker.copy_ instanceof Map) { + ;(changeTracker.copy_ as Map).set( + mapKey, + newValue + ) + } + }, + } + const { proxy: valueProxy } = memoizedCreateChangeProxy( + nextResult.value as Record, + mapParent + ) + nextResult.value = valueProxy + } + } else if (target instanceof Set) { + // For Set, track modifications + const setOriginalValue = nextResult.value + const setParent = { + tracker: changeTracker, + prop: setOriginalValue, + updateSet: (newValue: unknown) => { + if (changeTracker.copy_ instanceof Set) { + ;(changeTracker.copy_ as Set).delete( + setOriginalValue + ) + ;(changeTracker.copy_ as Set).add(newValue) + originalToModifiedMap.set(setOriginalValue, newValue) + } + }, + } + const { proxy: valueProxy } = memoizedCreateChangeProxy( + nextResult.value as Record, + setParent + ) + nextResult.value = valueProxy + } else { + // For other cases, use a symbol placeholder + const tempKey = Symbol(`iterator-value`) + const { proxy: valueProxy } = memoizedCreateChangeProxy( + nextResult.value as Record, + { + tracker: changeTracker, + prop: tempKey, + } + ) + nextResult.value = valueProxy + } + } + } + + return nextResult + }, + [Symbol.iterator]() { + return this + }, + } + } + + return result + } +} + /** * Simple debug utility that only logs when debug mode is enabled * Set DEBUG to true in localStorage to enable debug logging @@ -585,24 +819,13 @@ export function createChangeProxy< // For Array methods that modify the array if (Array.isArray(ptarget)) { const methodName = prop.toString() - const modifyingMethods = new Set([ - `pop`, - `push`, - `shift`, - `unshift`, - `splice`, - `sort`, - `reverse`, - `fill`, - `copyWithin`, - ]) - - if (modifyingMethods.has(methodName)) { - return function (...args: Array) { - const result = value.apply(changeTracker.copy_, args) - markChanged(changeTracker) - return result - } + + if (ARRAY_MODIFYING_METHODS.has(methodName)) { + return createModifyingMethodHandler( + value, + changeTracker, + markChanged + ) } // Handle array iteration methods (find, filter, forEach, etc.) @@ -628,234 +851,27 @@ export function createChangeProxy< // For Map and Set methods that modify the collection if (ptarget instanceof Map || ptarget instanceof Set) { const methodName = prop.toString() - const modifyingMethods = new Set([ - `set`, - `delete`, - `clear`, - `add`, - `pop`, - `push`, - `shift`, - `unshift`, - `splice`, - `sort`, - `reverse`, - ]) - - if (modifyingMethods.has(methodName)) { - return function (...args: Array) { - const result = value.apply(changeTracker.copy_, args) - markChanged(changeTracker) - return result - } + + if (MAP_SET_MODIFYING_METHODS.has(methodName)) { + return createModifyingMethodHandler( + value, + changeTracker, + markChanged + ) } // Handle iterator methods for Map and Set - const iteratorMethods = new Set([ - `entries`, - `keys`, - `values`, - `forEach`, - Symbol.iterator, - ]) - - if (iteratorMethods.has(methodName) || prop === Symbol.iterator) { - return function (this: unknown, ...args: Array) { - const result = value.apply(changeTracker.copy_, args) - - // For forEach, we need to wrap the callback to track changes - if (methodName === `forEach`) { - const callback = args[0] - if (typeof callback === `function`) { - // Replace the original callback with our wrapped version - const wrappedCallback = function ( - this: unknown, - // eslint-disable-next-line - value: unknown, - key: unknown, - collection: unknown - ) { - // Call the original callback - const cbresult = callback.call( - this, - value, - key, - collection - ) - // Mark as changed since the callback might have modified the value - markChanged(changeTracker) - return cbresult - } - // Call forEach with our wrapped callback - return value.apply(ptarget, [ - wrappedCallback, - ...args.slice(1), - ]) - } - } - - // For iterators (entries, keys, values, Symbol.iterator) - if ( - methodName === `entries` || - methodName === `values` || - methodName === Symbol.iterator.toString() || - prop === Symbol.iterator - ) { - // If it's an iterator, we need to wrap the returned iterator - // to track changes when the values are accessed and potentially modified - const originalIterator = result - - // For values() iterator on Maps, we need to create a value-to-key mapping - const valueToKeyMap = new Map() - if (methodName === `values` && ptarget instanceof Map) { - // Build a mapping from value to key for reverse lookup - // Use the copy_ (which is the current state) to build the mapping - for (const [ - key, - mapValue, - ] of changeTracker.copy_.entries()) { - valueToKeyMap.set(mapValue, key) - } - } - - // For Set iterators, we need to create an original-to-modified mapping - const originalToModifiedMap = new Map() - if (ptarget instanceof Set) { - // Initialize with original values - for (const setValue of changeTracker.copy_.values()) { - originalToModifiedMap.set(setValue, setValue) - } - } - - // Create a proxy for the iterator that will mark changes when next() is called - return { - next() { - const nextResult = originalIterator.next() - - // If we have a value and it's an object, we need to track it - if ( - !nextResult.done && - nextResult.value && - typeof nextResult.value === `object` - ) { - // For entries, the value is a [key, value] pair - if ( - methodName === `entries` && - Array.isArray(nextResult.value) && - nextResult.value.length === 2 - ) { - // The value is at index 1 in the [key, value] pair - if ( - nextResult.value[1] && - typeof nextResult.value[1] === `object` - ) { - const mapKey = nextResult.value[0] - // Create a special parent tracker that knows how to update the Map - const mapParent = { - tracker: changeTracker, - prop: mapKey, - updateMap: (newValue: unknown) => { - // Update the Map in the copy - if (changeTracker.copy_ instanceof Map) { - changeTracker.copy_.set(mapKey, newValue) - } - }, - } - - // Create a proxy for the value and replace it in the result - const { proxy: valueProxy } = - memoizedCreateChangeProxy( - nextResult.value[1], - mapParent - ) - nextResult.value[1] = valueProxy - } - } else if ( - methodName === `values` || - methodName === Symbol.iterator.toString() || - prop === Symbol.iterator - ) { - // If the value is an object, create a proxy for it - if ( - typeof nextResult.value === `object` && - nextResult.value !== null - ) { - // For Map values(), try to find the key using our mapping - if ( - methodName === `values` && - ptarget instanceof Map - ) { - const mapKey = valueToKeyMap.get(nextResult.value) - if (mapKey !== undefined) { - // Create a special parent tracker for this Map value - const mapParent = { - tracker: changeTracker, - prop: mapKey, - updateMap: (newValue: unknown) => { - // Update the Map in the copy - if (changeTracker.copy_ instanceof Map) { - changeTracker.copy_.set(mapKey, newValue) - } - }, - } - - const { proxy: valueProxy } = - memoizedCreateChangeProxy( - nextResult.value, - mapParent - ) - nextResult.value = valueProxy - } - } else if (ptarget instanceof Set) { - // For Set, we need to track modifications and update the Set accordingly - const setOriginalValue = nextResult.value - const setParent = { - tracker: changeTracker, - prop: setOriginalValue, // Use the original value as the prop - updateSet: (newValue: unknown) => { - // Update the Set in the copy by removing old value and adding new one - if (changeTracker.copy_ instanceof Set) { - changeTracker.copy_.delete(setOriginalValue) - changeTracker.copy_.add(newValue) - // Update our mapping for future iterations - originalToModifiedMap.set( - setOriginalValue, - newValue - ) - } - }, - } - - const { proxy: valueProxy } = - memoizedCreateChangeProxy( - nextResult.value, - setParent - ) - nextResult.value = valueProxy - } else { - // For other cases, use a symbol as a placeholder - const tempKey = Symbol(`iterator-value`) - const { proxy: valueProxy } = - memoizedCreateChangeProxy(nextResult.value, { - tracker: changeTracker, - prop: tempKey, - }) - nextResult.value = valueProxy - } - } - } - } - - return nextResult - }, - [Symbol.iterator]() { - return this - }, - } - } - - return result - } + const iteratorHandler = createMapSetIteratorHandler( + methodName, + prop, + value, + ptarget, + changeTracker, + memoizedCreateChangeProxy, + markChanged + ) + if (iteratorHandler) { + return iteratorHandler } } return value.bind(ptarget) From e216523e56923ce34a3bab3ce6fc263e72fce8a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Nov 2025 16:03:42 +0000 Subject: [PATCH 12/13] fix: restore updateMap/updateSet callbacks in Map/Set iterator handler --- packages/db/src/proxy.ts | 42 +++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index b86de37c9..ca5a5d8d1 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -256,7 +256,10 @@ function createMapSetIteratorHandler( changeTracker: ChangeTracker, memoizedCreateChangeProxy: ( obj: Record, - parent?: Record + parent?: { + tracker: ChangeTracker> + prop: string | symbol + } ) => { proxy: Record }, markChanged: (tracker: ChangeTracker) => void ): ((...args: Array) => unknown) | undefined { @@ -340,8 +343,10 @@ function createMapSetIteratorHandler( ) { const mapKey = nextResult.value[0] const mapParent = { - tracker: changeTracker, - prop: mapKey, + tracker: changeTracker as unknown as ChangeTracker< + Record + >, + prop: mapKey as string | symbol, updateMap: (newValue: unknown) => { if (changeTracker.copy_ instanceof Map) { ;(changeTracker.copy_ as Map).set( @@ -353,7 +358,10 @@ function createMapSetIteratorHandler( } const { proxy: valueProxy } = memoizedCreateChangeProxy( nextResult.value[1] as Record, - mapParent + mapParent as unknown as { + tracker: ChangeTracker> + prop: string | symbol + } ) nextResult.value[1] = valueProxy } @@ -367,8 +375,10 @@ function createMapSetIteratorHandler( const mapKey = valueToKeyMap.get(nextResult.value) if (mapKey !== undefined) { const mapParent = { - tracker: changeTracker, - prop: mapKey, + tracker: changeTracker as unknown as ChangeTracker< + Record + >, + prop: mapKey as string | symbol, updateMap: (newValue: unknown) => { if (changeTracker.copy_ instanceof Map) { ;(changeTracker.copy_ as Map).set( @@ -380,7 +390,10 @@ function createMapSetIteratorHandler( } const { proxy: valueProxy } = memoizedCreateChangeProxy( nextResult.value as Record, - mapParent + mapParent as unknown as { + tracker: ChangeTracker> + prop: string | symbol + } ) nextResult.value = valueProxy } @@ -388,8 +401,10 @@ function createMapSetIteratorHandler( // For Set, track modifications const setOriginalValue = nextResult.value const setParent = { - tracker: changeTracker, - prop: setOriginalValue, + tracker: changeTracker as unknown as ChangeTracker< + Record + >, + prop: setOriginalValue as unknown as string | symbol, updateSet: (newValue: unknown) => { if (changeTracker.copy_ instanceof Set) { ;(changeTracker.copy_ as Set).delete( @@ -402,7 +417,10 @@ function createMapSetIteratorHandler( } const { proxy: valueProxy } = memoizedCreateChangeProxy( nextResult.value as Record, - setParent + setParent as unknown as { + tracker: ChangeTracker> + prop: string | symbol + } ) nextResult.value = valueProxy } else { @@ -411,7 +429,9 @@ function createMapSetIteratorHandler( const { proxy: valueProxy } = memoizedCreateChangeProxy( nextResult.value as Record, { - tracker: changeTracker, + tracker: changeTracker as unknown as ChangeTracker< + Record + >, prop: tempKey, } ) From d52f46bb2986b5736ff4ea3ce47d66987426ca4e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Nov 2025 16:06:00 +0000 Subject: [PATCH 13/13] docs: update changeset to mention refactoring --- .changeset/fix-array-iteration-proxy.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/fix-array-iteration-proxy.md b/.changeset/fix-array-iteration-proxy.md index 449cb0935..34cdfa11b 100644 --- a/.changeset/fix-array-iteration-proxy.md +++ b/.changeset/fix-array-iteration-proxy.md @@ -16,3 +16,5 @@ collection.update(id, (draft) => { ``` The fix adds proxy handling for array iteration methods similar to how Map/Set iteration is already handled, ensuring that callbacks receive proxied elements and returned elements are properly proxied. + +Also refactors proxy.ts for improved readability by extracting helper functions and hoisting constants to module scope.