Skip to content

Conversation

@mtmacdonald
Copy link

Fixes #65. Adds a unit test with the schema.LoroTree example from the README docs, and resolves making the example code pass. Previously I got TypeError: Cannot read properties of undefined (reading 'push') when trying to invoke the example.

Would be great if I could have a review on this (I'm not too deep into the internals of this repo), and feedback is welcome if there's a better resolution (like fixing the docs example). Thank you!

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +441 to 446
const required =
schema.options.required !== undefined
? schema.options.required
: true;
const value = required ? [] : undefined;
if (value === undefined) return undefined;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid defaulting empty tree when snapshot omits it

This change makes getDefaultValue return [] for loro-tree when required is unset, which means Mirror's in-memory state will always include an empty tree even if the doc has no tree nodes. However, buildRootStateSnapshot only includes trees when they are non‑empty, so when checkStateConsistency is enabled, any setState call that doesn’t add tree nodes will now throw State diverged because the state has tree: [] but the snapshot omits it. Consider either keeping the default undefined for empty trees or updating the snapshot logic to include empty trees so the invariants still hold.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Advanced: Trees example from README throws TypError: Cannot read properties of undefined (reading 'push')

1 participant