fix(mirror): treat undefined values in Map as non-existent fields #66
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When setting state with objects containing undefined values like
{ name: 'test', age: undefined }, the undefined fields should be
treated as if they don't exist, rather than being converted to null.
Changes:
- diffMap: Skip undefined values when iterating newStateObj, and
generate delete changes if the old value existed
- initializeContainer: Skip undefined values when initializing Map
- stripUndefined: New utility function to recursively remove undefined
values from objects while preserving non-enumerable properties ()
- setState: Apply stripUndefined to newState before storing
This prevents state divergence errors when users pass objects with
undefined values to setState.
Co-authored-by: lody <agent@lody.ai>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Use Object.create(null) instead of {} to create result objects
- Skip unsafe keys (__proto__, constructor, prototype)
- Use Object.defineProperty instead of direct assignment for copying values
- Use Map instead of plain object for strippedValues to avoid pollution
This prevents attackers from polluting Object.prototype via JSON input
containing __proto__ or similar keys.
Co-authored-by: lody <agent@lody.ai>
Co-authored-by: lody <agent@lody.ai>
- Assert that undefined fields do not exist in state using 'key in obj' - Assert that accessing undefined fields returns undefined - Add test case for deleting existing field by setting to undefined Co-authored-by: lody <agent@lody.ai>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When setting state with objects containing
undefinedvalues (e.g.,{ name: 'test', age: undefined }), the code would throw a "State diverged" error. This happened becauseundefinedwas being converted tonullwhen written to LoroDoc, but the in-memory state retained theundefinedvalue, causing a mismatch during consistency checks.Solution
Treat
undefinedvalues as non-existent fields - they should be ignored rather than being stored.Changes
packages/core/src/core/diff.ts(diffMap function)undefinedvalues when iteratingnewStateObjdeletechanges if the old state had a non-undefined value for that keypackages/core/src/core/mirror.ts(initializeContainer method)undefinedvalues when initializing Map containerspackages/core/src/core/utils.ts(new stripUndefined function)undefinedvalues from objects$cid)undefinedvalues are found (preserves reference equality)packages/core/src/core/mirror.ts(setState method)stripUndefinedtonewStatebefore storingNew test file:
packages/core/tests/undefined-in-map.test.tsschema.Any()fieldsTesting
All 212 tests pass.