diff --git a/.changeset/slow-phones-fix.md b/.changeset/slow-phones-fix.md new file mode 100644 index 000000000..83cf3de15 --- /dev/null +++ b/.changeset/slow-phones-fix.md @@ -0,0 +1,5 @@ +--- +'@tanstack/db': patch +--- + +Fix update() replacing untouched custom class instances with plain objects. diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index 57723e3cc..2cb5ee385 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -54,18 +54,27 @@ const MAP_SET_ITERATOR_METHODS = new Set([ `forEach`, ]) +function isPlainObject(value: object): boolean { + const prototype = Object.getPrototypeOf(value) + return prototype === Object.prototype || prototype === null +} + /** - * Check if a value is a proxiable object (not Date, RegExp, or Temporal) + * Check if a value can be safely proxied without changing its semantics. */ function isProxiableObject( value: unknown, ): value is Record { + if (value === null || typeof value !== `object`) { + return false + } + return ( - value !== null && - typeof value === `object` && - !((value as any) instanceof Date) && - !((value as any) instanceof RegExp) && - !isTemporal(value) + isPlainObject(value) || + Array.isArray(value) || + value instanceof Map || + value instanceof Set || + (ArrayBuffer.isView(value) && !(value instanceof DataView)) ) } @@ -589,7 +598,16 @@ function deepClone( return obj } - const clone = {} as Record + // Preserve non-plain objects by reference. Proxying or cloning an arbitrary + // class instance as a plain object strips its prototype and internal state. + if (!isPlainObject(obj)) { + return obj + } + + const clone = Object.create(Object.getPrototypeOf(obj)) as Record< + string | symbol, + unknown + > visited.set(obj as object, clone) for (const key in obj) { @@ -897,7 +915,7 @@ export function createChangeProxy< return value.bind(ptarget) } - // If the value is an object (but not Date, RegExp, or Temporal), create a proxy for it + // Proxy only values whose semantics are preserved by our draft handling. if (isProxiableObject(value)) { // Create a parent reference for the nested object const nestedParent = { diff --git a/packages/db/tests/collection.test.ts b/packages/db/tests/collection.test.ts index e96994db6..a12a0d565 100644 --- a/packages/db/tests/collection.test.ts +++ b/packages/db/tests/collection.test.ts @@ -253,6 +253,48 @@ describe(`Collection`, () => { }).toThrow(KeyUpdateNotAllowedError) }) + it(`should preserve untouched custom class instances during updates`, async () => { + class Money { + constructor(public cents: number) {} + } + + type Product = { + id: string + details: { name: string; price: Money } + } + + const price = new Money(500) + const collection = createCollection({ + id: `custom-class-update-test`, + getKey: (item) => item.id, + sync: { + sync: ({ begin, write, commit, markReady }) => { + begin() + write({ + type: `insert`, + value: { + id: `product-1`, + details: { name: `Widget`, price }, + }, + }) + commit() + markReady() + }, + }, + onUpdate: async () => {}, + }) + + await collection.stateWhenReady() + + collection.update(`product-1`, (draft) => { + draft.details.name = `Gadget` + }) + + const updated = collection.get(`product-1`) + expect(updated?.details.price).toBe(price) + expect(updated?.details.price).toBeInstanceOf(Money) + }) + it(`It shouldn't expose any state until the initial sync is finished`, () => { // Create a collection with a mock sync plugin createCollection<{ name: string }>({ diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index bbac0151a..8ff3ed11d 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -1370,6 +1370,23 @@ describe(`Proxy Library`, () => { age: 30, }) }) + + it(`should preserve untouched custom class instances in changed objects`, () => { + class Money { + constructor(public cents: number) {} + } + + const price = new Money(500) + const obj = { details: { name: `Widget`, price } } + + const changes = withChangeTracking(obj, (proxy) => { + proxy.details.name = `Gadget` + }) + + const changedDetails = changes.details as typeof obj.details + expect(changedDetails.price).toBe(price) + expect(changedDetails.price).toBeInstanceOf(Money) + }) }) describe(`withArrayChangeTracking`, () => {