Skip to content

Commit da27275

Browse files
committed
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.
1 parent 2d979a0 commit da27275

1 file changed

Lines changed: 176 additions & 0 deletions

File tree

packages/db/src/proxy.ts

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,182 @@ export function createChangeProxy<
411411
return result
412412
}
413413
}
414+
415+
// For Array methods that iterate with callbacks and may return elements
416+
// These need to pass proxied elements to callbacks and return proxied results
417+
const callbackIterationMethods = new Set([
418+
`find`,
419+
`findLast`,
420+
`findIndex`,
421+
`findLastIndex`,
422+
`filter`,
423+
`map`,
424+
`flatMap`,
425+
`forEach`,
426+
`some`,
427+
`every`,
428+
`reduce`,
429+
`reduceRight`,
430+
])
431+
432+
if (callbackIterationMethods.has(methodName)) {
433+
return function (...args: Array<unknown>) {
434+
const callback = args[0]
435+
if (typeof callback !== `function`) {
436+
return value.apply(changeTracker.copy_, args)
437+
}
438+
439+
// Create a helper to get proxied version of an array element
440+
const getProxiedElement = (
441+
element: unknown,
442+
index: number
443+
): unknown => {
444+
if (
445+
element &&
446+
typeof element === `object` &&
447+
!((element as any) instanceof Date) &&
448+
!((element as any) instanceof RegExp) &&
449+
!isTemporal(element)
450+
) {
451+
// Create a parent reference for the array element
452+
const nestedParent = {
453+
tracker: changeTracker,
454+
prop: String(index),
455+
}
456+
const { proxy: elementProxy } = memoizedCreateChangeProxy(
457+
element as Record<string | symbol, unknown>,
458+
nestedParent
459+
)
460+
return elementProxy
461+
}
462+
return element
463+
}
464+
465+
// Wrap the callback to pass proxied elements
466+
const wrappedCallback = function (
467+
this: unknown,
468+
element: unknown,
469+
index: number,
470+
array: unknown
471+
) {
472+
const proxiedElement = getProxiedElement(element, index)
473+
return callback.call(this, proxiedElement, index, array)
474+
}
475+
476+
// For reduce/reduceRight, the callback signature is different
477+
if (methodName === `reduce` || methodName === `reduceRight`) {
478+
const reduceCallback = function (
479+
this: unknown,
480+
accumulator: unknown,
481+
element: unknown,
482+
index: number,
483+
array: unknown
484+
) {
485+
const proxiedElement = getProxiedElement(element, index)
486+
return callback.call(
487+
this,
488+
accumulator,
489+
proxiedElement,
490+
index,
491+
array
492+
)
493+
}
494+
return value.apply(changeTracker.copy_, [
495+
reduceCallback,
496+
...args.slice(1),
497+
])
498+
}
499+
500+
const result = value.apply(changeTracker.copy_, [
501+
wrappedCallback,
502+
...args.slice(1),
503+
])
504+
505+
// For find/findLast, proxy the returned element if it's an object
506+
if (
507+
(methodName === `find` || methodName === `findLast`) &&
508+
result &&
509+
typeof result === `object`
510+
) {
511+
// Find the index of the result in the array
512+
const foundIndex = (
513+
changeTracker.copy_ as Array<unknown>
514+
).indexOf(result)
515+
if (foundIndex !== -1) {
516+
return getProxiedElement(result, foundIndex)
517+
}
518+
}
519+
520+
// For filter/map/flatMap, proxy each element in the result array
521+
if (
522+
(methodName === `filter` ||
523+
methodName === `map` ||
524+
methodName === `flatMap`) &&
525+
Array.isArray(result)
526+
) {
527+
// For filter, the result contains original elements
528+
// For map/flatMap, the result contains new elements from callback
529+
// We don't need to proxy map/flatMap results as they're new objects
530+
if (methodName === `filter`) {
531+
return result.map((element) => {
532+
const originalIndex = (
533+
changeTracker.copy_ as Array<unknown>
534+
).indexOf(element)
535+
if (originalIndex !== -1) {
536+
return getProxiedElement(element, originalIndex)
537+
}
538+
return element
539+
})
540+
}
541+
}
542+
543+
return result
544+
}
545+
}
546+
547+
// Handle array Symbol.iterator for for...of loops
548+
if (prop === Symbol.iterator) {
549+
return function () {
550+
const array = changeTracker.copy_ as Array<unknown>
551+
let index = 0
552+
553+
return {
554+
next() {
555+
if (index >= array.length) {
556+
return { done: true, value: undefined }
557+
}
558+
559+
const element = array[index]
560+
let proxiedElement = element
561+
562+
// Proxy object elements
563+
if (
564+
element &&
565+
typeof element === `object` &&
566+
!((element as any) instanceof Date) &&
567+
!((element as any) instanceof RegExp) &&
568+
!isTemporal(element)
569+
) {
570+
const nestedParent = {
571+
tracker: changeTracker,
572+
prop: String(index),
573+
}
574+
const { proxy: elementProxy } = memoizedCreateChangeProxy(
575+
element as Record<string | symbol, unknown>,
576+
nestedParent
577+
)
578+
proxiedElement = elementProxy
579+
}
580+
581+
index++
582+
return { done: false, value: proxiedElement }
583+
},
584+
[Symbol.iterator]() {
585+
return this
586+
},
587+
}
588+
}
589+
}
414590
}
415591

416592
// For Map and Set methods that modify the collection

0 commit comments

Comments
 (0)