Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds an optional inPlace parameter to the deserialize method that allows deserialization to mutate the original JSON object instead of creating a copy. The parse method is also updated to use in-place deserialization by default.
- Added optional
inPlaceparameter to thedeserializemethod - Modified
parsemethod to use in-place deserialization for performance optimization - Added test coverage for the new in-place deserialization functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/index.ts | Added inPlace option to deserialize method and updated parse to use it |
| src/index.test.ts | Added test case to verify in-place deserialization behavior |
| parse<T = unknown>(string: string): T { | ||
| return this.deserialize(JSON.parse(string)); | ||
| return this.deserialize(JSON.parse(string), { inPlace: true }); |
There was a problem hiding this comment.
Changing the parse method to use inPlace: true by default is a breaking change that could affect existing users who rely on the original JSON object remaining unmodified. Consider making this configurable or documenting this behavior change prominently.
There was a problem hiding this comment.
I don't think so, Copilot. JSON.parse is creating the object and it's safe to mutate it in place, because no other caller has access to it.
flybayer
left a comment
flybayer
left a comment
There was a problem hiding this comment.
looks good, just need do update the API docs
|
|
|
I was meaning we should document the |
|
That's what I did, no? |
2 participants
Closes #319