Skip to content

Commit 665efe6

Browse files
KyleAMathewsclaude
andauthored
Add support for array mutating methods in proxy change tracking (#267)
Co-authored-by: Claude <[email protected]>
1 parent fe42591 commit 665efe6

File tree

3 files changed

+112
-18
lines changed

3 files changed

+112
-18
lines changed

.changeset/tasty-walls-type.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@tanstack/db": patch
3+
---
4+
5+
• Add proper tracking for array mutating methods (push, pop, shift, unshift, splice, sort, reverse, fill, copyWithin)
6+
• Fix existing array tests that were misleadingly named but didn't actually call the methods they claimed to test
7+
• Add comprehensive test coverage for all supported array mutating methods

packages/db/src/proxy.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,30 @@ export function createChangeProxy<
461461

462462
// If the value is a function, bind it to the ptarget
463463
if (typeof value === `function`) {
464+
// For Array methods that modify the array
465+
if (Array.isArray(ptarget)) {
466+
const methodName = prop.toString()
467+
const modifyingMethods = new Set([
468+
`pop`,
469+
`push`,
470+
`shift`,
471+
`unshift`,
472+
`splice`,
473+
`sort`,
474+
`reverse`,
475+
`fill`,
476+
`copyWithin`,
477+
])
478+
479+
if (modifyingMethods.has(methodName)) {
480+
return function (...args: Array<unknown>) {
481+
const result = value.apply(changeTracker.copy_, args)
482+
markChanged(changeTracker)
483+
return result
484+
}
485+
}
486+
}
487+
464488
// For Map and Set methods that modify the collection
465489
if (ptarget instanceof Map || ptarget instanceof Set) {
466490
const methodName = prop.toString()

packages/db/tests/proxy.test.ts

Lines changed: 81 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -728,9 +728,9 @@ describe(`Proxy Library`, () => {
728728
const objs = [{ items: [`apple`, `banana`, `cherry`] }]
729729
const { proxies, getChanges } = createArrayChangeProxy(objs)
730730

731-
// Create a new array without the last element
731+
// Call pop() method directly
732732
// @ts-expect-error ok possibly undefined
733-
proxies[0].items = proxies[0].items.slice(0, -1)
733+
proxies[0].items.pop()
734734

735735
expect(getChanges()).toEqual([
736736
{
@@ -745,10 +745,9 @@ describe(`Proxy Library`, () => {
745745
const objs = [{ items: [`apple`, `banana`, `cherry`] }]
746746
const { proxies, getChanges } = createArrayChangeProxy(objs)
747747

748-
// Create a new array without the first element
749-
748+
// Call shift() method directly
750749
// @ts-expect-error ok possibly undefined
751-
proxies[0].items = proxies[0].items.slice(1)
750+
proxies[0].items.shift()
752751

753752
expect(getChanges()).toEqual([
754753
{
@@ -763,33 +762,38 @@ describe(`Proxy Library`, () => {
763762
const objs = [{ items: [`banana`, `cherry`] }]
764763
const { proxies, getChanges } = createArrayChangeProxy(objs)
765764

766-
// Create a new array with an element added at the beginning
765+
// Call unshift() method directly
767766
// @ts-expect-error ok possibly undefined
768-
769-
proxies[0].items = [`apple`, ...proxies[0].items]
767+
proxies[0].items.unshift(`apple`)
770768

771769
expect(getChanges()).toEqual([
772770
{
773771
items: [`apple`, `banana`, `cherry`],
774772
},
775773
])
776774
// @ts-expect-error ok possibly undefined
777-
778775
expect(objs[0].items).toEqual([`banana`, `cherry`])
779776
})
780777

778+
it(`should track array push() operations`, () => {
779+
const obj = { items: [`apple`, `banana`] }
780+
const { proxy, getChanges } = createChangeProxy(obj)
781+
782+
proxy.items.push(`cherry`)
783+
784+
expect(getChanges()).toEqual({
785+
items: [`apple`, `banana`, `cherry`],
786+
})
787+
expect(obj.items).toEqual([`apple`, `banana`])
788+
})
789+
781790
it(`should track array splice() operations`, () => {
782791
const objs = [{ items: [`apple`, `banana`, `cherry`, `date`] }]
783792
const { proxies, getChanges } = createArrayChangeProxy(objs)
784793

785-
// Create a new array with elements replaced in the middle
786-
// @ts-expect-error ok possibly undefined
787-
788-
const newItems = [...proxies[0].items]
789-
newItems.splice(1, 2, `blueberry`, `cranberry`)
794+
// Call splice() method directly
790795
// @ts-expect-error ok possibly undefined
791-
792-
proxies[0].items = newItems
796+
proxies[0].items.splice(1, 2, `blueberry`, `cranberry`)
793797

794798
expect(getChanges()).toEqual([
795799
{
@@ -804,9 +808,9 @@ describe(`Proxy Library`, () => {
804808
const objs = [{ items: [`cherry`, `apple`, `banana`] }]
805809
const { proxies, getChanges } = createArrayChangeProxy(objs)
806810

807-
// Create a new sorted array
811+
// Call sort() method directly
808812
// @ts-expect-error ok possibly undefined
809-
proxies[0].items = [...proxies[0].items].sort()
813+
proxies[0].items.sort()
810814

811815
expect(getChanges()).toEqual([
812816
{
@@ -817,6 +821,65 @@ describe(`Proxy Library`, () => {
817821
expect(objs[0].items).toEqual([`cherry`, `apple`, `banana`])
818822
})
819823

824+
it(`should track array reverse() operations`, () => {
825+
const objs = [{ items: [`apple`, `banana`, `cherry`] }]
826+
const { proxies, getChanges } = createArrayChangeProxy(objs)
827+
828+
// Call reverse() method directly
829+
// @ts-expect-error ok possibly undefined
830+
proxies[0].items.reverse()
831+
832+
expect(getChanges()).toEqual([
833+
{
834+
items: [`cherry`, `banana`, `apple`],
835+
},
836+
])
837+
// @ts-expect-error ok possibly undefined
838+
expect(objs[0].items).toEqual([`apple`, `banana`, `cherry`])
839+
})
840+
841+
it(`should track array fill() operations`, () => {
842+
const objs = [{ items: [`apple`, `banana`, `cherry`] }]
843+
const { proxies, getChanges } = createArrayChangeProxy(objs)
844+
845+
// Call fill() method directly
846+
// @ts-expect-error ok possibly undefined
847+
proxies[0].items.fill(`orange`, 1, 3)
848+
849+
expect(getChanges()).toEqual([
850+
{
851+
items: [`apple`, `orange`, `orange`],
852+
},
853+
])
854+
// @ts-expect-error ok possibly undefined
855+
expect(objs[0].items).toEqual([`apple`, `banana`, `cherry`])
856+
})
857+
858+
it(`should track array copyWithin() operations`, () => {
859+
const objs = [
860+
{ items: [`apple`, `banana`, `cherry`, `date`, `elderberry`] },
861+
]
862+
const { proxies, getChanges } = createArrayChangeProxy(objs)
863+
864+
// Call copyWithin() method directly - copy elements from index 3-4 to index 0-1
865+
// @ts-expect-error ok possibly undefined
866+
proxies[0].items.copyWithin(0, 3, 5)
867+
868+
expect(getChanges()).toEqual([
869+
{
870+
items: [`date`, `elderberry`, `cherry`, `date`, `elderberry`],
871+
},
872+
])
873+
// @ts-expect-error ok possibly undefined
874+
expect(objs[0].items).toEqual([
875+
`apple`,
876+
`banana`,
877+
`cherry`,
878+
`date`,
879+
`elderberry`,
880+
])
881+
})
882+
820883
it(`should track changes in multi-dimensional arrays`, () => {
821884
const objs = [
822885
{

0 commit comments

Comments
 (0)