From 3d57e2f8cca9bdc0b56cb23e6122316c5d7dd70d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Nov 2025 19:42:50 +0000 Subject: [PATCH 1/3] fix(db): handle frozen objects in proxy Use the unfrozen internal copy as the Proxy target instead of the original frozen object. This fixes a TypeError that occurred when trying to modify properties on a proxy wrapping a frozen object (such as state from libraries that freeze their data). Added comprehensive tests for frozen object handling including deeply frozen nested objects and array change tracking. --- .changeset/fix-proxy-frozen-values.md | 5 + packages/db/src/proxy.ts | 4 +- packages/db/tests/proxy.test.ts | 290 +++++++++++++++----------- 3 files changed, 179 insertions(+), 120 deletions(-) create mode 100644 .changeset/fix-proxy-frozen-values.md diff --git a/.changeset/fix-proxy-frozen-values.md b/.changeset/fix-proxy-frozen-values.md new file mode 100644 index 000000000..45a46304a --- /dev/null +++ b/.changeset/fix-proxy-frozen-values.md @@ -0,0 +1,5 @@ +--- +"@tanstack/db": patch +--- + +Fix proxy to handle frozen objects correctly. Previously, creating a proxy for a frozen object (such as data from state management libraries that freeze their state) would throw a TypeError when attempting to modify properties via the proxy. The proxy now uses an unfrozen internal copy as the Proxy target, allowing modifications to be tracked correctly while preserving the immutability of the original object. diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index ca5a5d8d1..58251aeda 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -1060,7 +1060,9 @@ export function createChangeProxy< } // Create a proxy for the target object - const proxy = createObjectProxy(target) + // Use the unfrozen copy_ as the proxy target to avoid Proxy invariant violations + // when the original target is frozen (e.g., from Immer) + const proxy = createObjectProxy(changeTracker.copy_ as unknown as T) // Return the proxy and a function to get the changes return { diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index 41bb0fee5..c7c08eda2 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -360,125 +360,177 @@ describe(`Proxy Library`, () => { // }) }) - // describe(`Object.freeze and Object.seal handling`, () => { - // it(`should handle Object.freeze correctly`, () => { - // const obj = { name: `John`, age: 30 } - // const { proxy, getChanges } = createChangeProxy(obj) - // - // // Freeze the proxy - // Object.freeze(proxy) - // - // // Attempt to modify the frozen proxy (should throw in strict mode) - // let errorThrown = false - // let errorMessage = `` - // try { - // proxy.name = `Jane` - // } catch (e) { - // // Expected error - // errorThrown = true - // errorMessage = e instanceof Error ? e.message : String(e) - // } - // - // // In strict mode, an error should be thrown - // if (errorThrown) { - // // Verify the error message contains expected text about the property being read-only - // expect(errorMessage).toContain(`read only property`) - // } - // - // // Either way, no changes should be tracked - // expect(getChanges()).toEqual({}) - // - // // Check that the original object is unchanged - // expect(obj).toEqual({ - // name: `John`, - // age: 30, - // }) - // }) - - // it(`should handle Object.seal correctly`, () => { - // const obj = { name: `John`, age: 30 } - // const { proxy, getChanges } = createChangeProxy(obj) - // - // // Seal the proxy - // Object.seal(proxy) - // - // // Modify existing property (should work) - // proxy.name = `Jane` - // - // // Attempt to add a new property (should not work) - // let errorThrown = false - // let errorMessage = `` - // try { - // // @ts-expect-error ignore for test - // proxy.role = `admin` - // } catch (e) { - // // Expected error - // errorThrown = true - // errorMessage = e instanceof Error ? e.message : String(e) - // } - // - // // In strict mode, an error should be thrown - // if (errorThrown) { - // // Verify the error message contains expected text about the object not being extensible - // expect(errorMessage).toContain(`object is not extensible`) - // } - // - // // Check that only the name change was tracked - // expect(getChanges()).toEqual({ - // name: `Jane`, - // }) - // - // // Check that the original object has the name change but no new property - // expect(obj).toEqual({ - // name: `Jane`, - // age: 30, - // }) - // - // expect(obj.hasOwnProperty(`role`)).toBe(false) - // }) - - // it(`should handle Object.preventExtensions correctly`, () => { - // const obj = { name: `John`, age: 30 } - // const { proxy, getChanges } = createChangeProxy(obj) - // - // // Prevent extensions on the proxy - // Object.preventExtensions(proxy) - // - // // Modify existing property (should work) - // proxy.name = `Jane` - // - // // Attempt to add a new property (should not work) - // let errorThrown = false - // let errorMessage = `` - // try { - // // @ts-expect-error ignore for test - // proxy.role = `admin` - // } catch (e) { - // // Expected error - // errorThrown = true - // errorMessage = e instanceof Error ? e.message : String(e) - // } - // - // // In strict mode, an error should be thrown - // if (errorThrown) { - // // Verify the error message contains expected text about the object not being extensible - // expect(errorMessage).toContain(`object is not extensible`) - // } - // - // // Check that only the name change was tracked - // expect(getChanges()).toEqual({ - // name: `Jane`, - // }) - // - // // Check that the original object has the name change but no new property - // expect(obj).toEqual({ - // name: `Jane`, - // age: 30, - // }) - // - // expect(obj.hasOwnProperty(`role`)).toBe(false) - // }) - // }) + describe(`Frozen object handling`, () => { + it(`should handle creating a proxy for an already-frozen object`, () => { + const obj = { name: `John`, age: 30 } + Object.freeze(obj) + + // This should not throw - the proxy should work with frozen objects + const { proxy, getChanges } = createChangeProxy(obj) + + // Modify properties via the proxy + proxy.name = `Jane` + proxy.age = 31 + + // Changes should be tracked + expect(getChanges()).toEqual({ + name: `Jane`, + age: 31, + }) + + // Original frozen object should remain unchanged + expect(obj).toEqual({ + name: `John`, + age: 30, + }) + expect(Object.isFrozen(obj)).toBe(true) + }) + + it(`should handle deeply frozen nested objects`, () => { + const obj = { + user: { + name: `John`, + address: { + city: `NYC`, + zip: `10001`, + }, + }, + } + // Deep freeze the object + Object.freeze(obj) + Object.freeze(obj.user) + Object.freeze(obj.user.address) + + const { proxy, getChanges } = createChangeProxy(obj) + + // Modify nested properties + proxy.user.name = `Jane` + proxy.user.address.city = `LA` + + // Changes should be tracked + expect(getChanges()).toEqual({ + user: { + name: `Jane`, + address: { + city: `LA`, + zip: `10001`, + }, + }, + }) + + // Original should be unchanged + expect(obj.user.name).toBe(`John`) + expect(obj.user.address.city).toBe(`NYC`) + }) + + it(`should handle withArrayChangeTracking with frozen objects`, () => { + const item1 = { id: 1, name: `Item 1` } + const item2 = { id: 2, name: `Item 2` } + Object.freeze(item1) + Object.freeze(item2) + const frozenArray = [item1, item2] + Object.freeze(frozenArray) + + // This should not throw - matches the RTK Query adapter use case + const changes = withArrayChangeTracking(frozenArray, (drafts) => { + if (drafts[0]) { + drafts[0].name = `Updated Item 1` + } + }) + + // Changes should be captured + expect(changes[0]).toEqual({ name: `Updated Item 1` }) + expect(changes[1]).toEqual({}) + + // Original frozen objects should be unchanged + expect(item1.name).toBe(`Item 1`) + expect(Object.isFrozen(item1)).toBe(true) + }) + + it(`should handle withChangeTracking with a frozen object`, () => { + const obj = { id: 1, name: `Test`, value: 100 } + Object.freeze(obj) + + const changes = withChangeTracking(obj, (draft) => { + draft.name = `Updated` + draft.value = 200 + }) + + expect(changes).toEqual({ + name: `Updated`, + value: 200, + }) + + // Original should be unchanged + expect(obj.name).toBe(`Test`) + expect(obj.value).toBe(100) + }) + + it(`should handle frozen arrays with nested frozen objects`, () => { + const data = [ + { id: 1, details: { score: 10 } }, + { id: 2, details: { score: 20 } }, + ] + // Deep freeze everything + data.forEach((item) => { + Object.freeze(item.details) + Object.freeze(item) + }) + Object.freeze(data) + + const changes = withArrayChangeTracking(data, (drafts) => { + // Modify nested property + if (drafts[0]) { + drafts[0].details.score = 100 + } + }) + + expect(changes[0]).toEqual({ + details: { score: 100 }, + }) + + // Original should be unchanged + expect(data[0]!.details.score).toBe(10) + }) + + it(`should handle iteration over frozen array elements`, () => { + const items = [ + { id: 1, name: `A` }, + { id: 2, name: `B` }, + { id: 3, name: `C` }, + ] + items.forEach((item) => Object.freeze(item)) + Object.freeze(items) + + const changes = withArrayChangeTracking(items, (drafts) => { + // Use find to locate and modify an item + const found = drafts.find((d) => d.id === 2) + if (found) { + found.name = `Updated B` + } + }) + + expect(changes[1]).toEqual({ name: `Updated B` }) + expect(items[1]!.name).toBe(`B`) // Original unchanged + }) + + it(`should handle createArrayChangeProxy with frozen objects`, () => { + const items = [ + { id: 1, status: `pending` }, + { id: 2, status: `pending` }, + ] + items.forEach((item) => Object.freeze(item)) + + const { proxies, getChanges } = createArrayChangeProxy(items) + + proxies[0]!.status = `completed` + proxies[1]!.status = `in-progress` + + const changes = getChanges() + expect(changes[0]).toEqual({ status: `completed` }) + expect(changes[1]).toEqual({ status: `in-progress` }) + }) + }) describe(`Enhanced Iterator Method Tracking`, () => { it(`should track changes when Map values are modified via iterator`, () => { From 2adf2407bf37060e1d60b3454e85a903de308118 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Dec 2025 14:28:40 +0000 Subject: [PATCH 2/3] fix(db): add Object.seal and Object.preventExtensions support to proxy - Add proper Proxy traps for defineProperty, getOwnPropertyDescriptor, preventExtensions, and isExtensible that forward to the target - Update deleteProperty trap to use Reflect.deleteProperty - Add comprehensive tests for seal and preventExtensions behavior - Update changeset to document the new functionality --- .changeset/fix-proxy-frozen-values.md | 2 + packages/db/src/proxy.ts | 79 +++++++++++-------- packages/db/tests/proxy.test.ts | 108 ++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 33 deletions(-) diff --git a/.changeset/fix-proxy-frozen-values.md b/.changeset/fix-proxy-frozen-values.md index 45a46304a..a84b8c117 100644 --- a/.changeset/fix-proxy-frozen-values.md +++ b/.changeset/fix-proxy-frozen-values.md @@ -3,3 +3,5 @@ --- Fix proxy to handle frozen objects correctly. Previously, creating a proxy for a frozen object (such as data from state management libraries that freeze their state) would throw a TypeError when attempting to modify properties via the proxy. The proxy now uses an unfrozen internal copy as the Proxy target, allowing modifications to be tracked correctly while preserving the immutability of the original object. + +Also adds support for `Object.seal()` and `Object.preventExtensions()` on proxies, allowing these operations to work correctly on change-tracking proxies. diff --git a/packages/db/src/proxy.ts b/packages/db/src/proxy.ts index 58251aeda..04cd84a71 100644 --- a/packages/db/src/proxy.ts +++ b/packages/db/src/proxy.ts @@ -994,21 +994,31 @@ export function createChangeProxy< return true }, - defineProperty(_ptarget, prop, descriptor) { - // const result = Reflect.defineProperty( - // changeTracker.copy_, - // prop, - // descriptor - // ) - // if (result) { - if (`value` in descriptor) { + defineProperty(ptarget, prop, descriptor) { + // Forward the defineProperty to the target to maintain Proxy invariants + // This allows Object.seal() and Object.freeze() to work on the proxy + const result = Reflect.defineProperty(ptarget, prop, descriptor) + if (result && `value` in descriptor) { changeTracker.copy_[prop as keyof T] = deepClone(descriptor.value) changeTracker.assigned_[prop.toString()] = true markChanged(changeTracker) } - // } - // return result - return true + return result + }, + + getOwnPropertyDescriptor(ptarget, prop) { + // Forward to target to maintain Proxy invariants for seal/freeze + return Reflect.getOwnPropertyDescriptor(ptarget, prop) + }, + + preventExtensions(ptarget) { + // Forward to target to allow Object.seal() and Object.preventExtensions() + return Reflect.preventExtensions(ptarget) + }, + + isExtensible(ptarget) { + // Forward to target to maintain consistency + return Reflect.isExtensible(ptarget) }, deleteProperty(dobj, prop) { @@ -1020,33 +1030,36 @@ export function createChangeProxy< const hadPropertyInOriginal = stringProp in changeTracker.originalObject - // Delete the property from the copy - // Use type assertion to tell TypeScript this is allowed - delete (changeTracker.copy_ as Record)[prop] + // Forward the delete to the target using Reflect + // This respects Object.seal/preventExtensions constraints + const result = Reflect.deleteProperty(dobj, prop) - // If the property didn't exist in the original object, removing it - // should revert to the original state - if (!hadPropertyInOriginal) { - delete changeTracker.copy_[stringProp] - delete changeTracker.assigned_[stringProp] + if (result) { + // If the property didn't exist in the original object, removing it + // should revert to the original state + if (!hadPropertyInOriginal) { + delete changeTracker.assigned_[stringProp] - // If this is the last change and we're not a nested object, - // mark the object as unmodified - if ( - Object.keys(changeTracker.assigned_).length === 0 && - Object.getOwnPropertySymbols(changeTracker.assigned_).length === 0 - ) { - changeTracker.modified = false + // If this is the last change and we're not a nested object, + // mark the object as unmodified + if ( + Object.keys(changeTracker.assigned_).length === 0 && + Object.getOwnPropertySymbols(changeTracker.assigned_).length === + 0 + ) { + changeTracker.modified = false + } else { + // We still have changes, keep as modified + changeTracker.modified = true + } } else { - // We still have changes, keep as modified - changeTracker.modified = true + // Mark this property as deleted + changeTracker.assigned_[stringProp] = false + markChanged(changeTracker) } - } else { - // Mark this property as deleted - changeTracker.assigned_[stringProp] = false - changeTracker.copy_[stringProp as keyof T] = undefined as T[keyof T] - markChanged(changeTracker) } + + return result } return true diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index c7c08eda2..966642565 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -532,6 +532,114 @@ describe(`Proxy Library`, () => { }) }) + describe(`Object.seal and Object.preventExtensions handling`, () => { + it(`should handle Object.seal correctly`, () => { + const obj = { name: `John`, age: 30 } + const { proxy, getChanges } = createChangeProxy(obj) + + // Seal the proxy + Object.seal(proxy) + + // Modify existing property (should work) + proxy.name = `Jane` + + // Attempt to add a new property (should throw in strict mode) + expect(() => { + // @ts-expect-error testing runtime behavior + proxy.role = `admin` + }).toThrow(/not extensible/) + + // Check that only the name change was tracked + expect(getChanges()).toEqual({ + name: `Jane`, + }) + + // Original object should be unchanged + expect(obj).toEqual({ + name: `John`, + age: 30, + }) + }) + + it(`should handle Object.preventExtensions correctly`, () => { + const obj = { name: `John`, age: 30 } + const { proxy, getChanges } = createChangeProxy(obj) + + // Prevent extensions on the proxy + Object.preventExtensions(proxy) + + // Modify existing property (should work) + proxy.name = `Jane` + + // Attempt to add a new property (should throw) + expect(() => { + // @ts-expect-error testing runtime behavior + proxy.role = `admin` + }).toThrow(/not extensible/) + + // Check that only the name change was tracked + expect(getChanges()).toEqual({ + name: `Jane`, + }) + + // Original object should be unchanged + expect(obj).toEqual({ + name: `John`, + age: 30, + }) + }) + + it(`should allow deleting properties with preventExtensions (but not seal)`, () => { + const obj = { name: `John`, age: 30 } + const { proxy, getChanges } = createChangeProxy(obj) + + // Object.preventExtensions allows deletion of configurable properties + // Object.seal makes properties non-configurable, so delete wouldn't work + Object.preventExtensions(proxy) + + // Modify and delete + proxy.name = `Jane` + delete proxy.age + + // Only the modified property is returned by getChanges + // (deleted properties are tracked internally but not in the result) + const changes = getChanges() + expect(changes.name).toBe(`Jane`) + }) + + it(`should not allow deleting properties on sealed objects`, () => { + const obj = { name: `John`, age: 30 } + const { proxy } = createChangeProxy(obj) + + // Object.seal makes properties non-configurable + Object.seal(proxy) + + // In strict mode (which Vitest uses), deleting a non-configurable property throws + expect(() => { + delete proxy.age + }).toThrow() + + // Property should still exist + expect(proxy.age).toBe(30) + }) + + it(`should handle sealing a proxy with nested objects`, () => { + const obj = { user: { name: `John` }, count: 0 } + const { proxy, getChanges } = createChangeProxy(obj) + + Object.seal(proxy) + + // Modifying nested objects should still work + proxy.user.name = `Jane` + proxy.count = 5 + + expect(getChanges()).toEqual({ + user: { name: `Jane` }, + count: 5, + }) + }) + }) + describe(`Enhanced Iterator Method Tracking`, () => { it(`should track changes when Map values are modified via iterator`, () => { const map = new Map([ From d39e7330c1d4acf5d4384cab3477b5ff3b25d6de Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Dec 2025 15:24:18 +0000 Subject: [PATCH 3/3] fix(db): add ts-expect-error for delete operator type checks --- packages/db/tests/proxy.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/db/tests/proxy.test.ts b/packages/db/tests/proxy.test.ts index 966642565..2f7cbff1b 100644 --- a/packages/db/tests/proxy.test.ts +++ b/packages/db/tests/proxy.test.ts @@ -599,6 +599,7 @@ describe(`Proxy Library`, () => { // Modify and delete proxy.name = `Jane` + // @ts-expect-error testing delete on non-optional property delete proxy.age // Only the modified property is returned by getChanges @@ -616,6 +617,7 @@ describe(`Proxy Library`, () => { // In strict mode (which Vitest uses), deleting a non-configurable property throws expect(() => { + // @ts-expect-error testing delete on non-optional property delete proxy.age }).toThrow()