-
Notifications
You must be signed in to change notification settings - Fork 25
fix(apply): fix apply with patches non-string keys #151
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
Conversation
|
Coverage after merging apply-patches into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Pull Request Overview
This PR fixes issues with applying patches when dealing with non-string keys in objects and maps, specifically handling object keys, symbol keys, and proper key coercion for intermediate paths.
- Enhanced key handling logic to properly support symbol keys and object keys in patch paths
- Added safety checks to prevent patching reserved attributes like
__proto__andconstructorwith non-string keys - Implemented proper key coercion for non-primitive keys when accessing object properties
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/apply.ts | Updated key handling logic to support symbols and properly coerce non-primitive keys |
| test/index.test.ts | Added test cases for map with object keys, symbol keys on objects, and key coercion |
| test/immer-non-support.test.ts | Added comprehensive test coverage comparing behavior between applyPatches and apply functions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Coverage after merging apply-patches into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#150