-
Notifications
You must be signed in to change notification settings - Fork 132
Fix handling of frozen objects in proxy #933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix handling of frozen objects in proxy #933
Conversation
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 detectedLatest commit: d39e733 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: +57 B (+0.07%) Total Size: 87.1 kB
ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 3.34 kB ℹ️ View Unchanged
|
samwillis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good.
If I understand correctly, we make a deep copy of the target, and now use that as the proxy target rather than the original source object.
Note that the old tests that where commented out also tested Object.seal, but the new tests do not, they only test Object.freeze
- 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
|
Yeah exactly. We were already making a clone and should have been using it. And yeah good call, also added support for .seal/.preventExtensions. This is a good test of the improved models — it went in circles on this ~6 months ago when I first tried implementing this but Claude 4.5 fixed it immediately this go around. |
|
🎉 This PR has been released! Thank you for your contribution! |
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.
🎯 Changes
✅ Checklist
pnpm test:pr.🚀 Release Impact