feat(sync): Add checks when containerId is provided#67
Open
kulla wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds validation checks when a containerId is provided to updateLoroToPmState() to prevent Uncaught TypeError exceptions when dealing with detached or invalid containers. The changes improve error handling and developer experience by providing clear error messages.
Key Changes:
- Extracted container retrieval logic into a dedicated
getContainer()helper function with proper validation - Added null check for non-existent containers with helpful error message
- Added type validation to ensure the container is a
LoroMapinstance
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function valueToAttrs( | ||
| value: Value | Attrs | null | undefined, | ||
| ): Attrs | null { | ||
| function valueToAttrs(value: Value | Attrs | null | undefined): Attrs | null { |
There was a problem hiding this comment.
This formatting change appears unrelated to the PR's stated purpose of adding checks for containerId validation. Consider keeping formatting changes in separate commits for clearer git history.
I run into an issue providing a container ID of an detached container. With the implementation before I got an `Uncaught TypeError` since there is no type check whether `getContainerId()` returns a container so that `map.get()` cannot be called. I took me a while to find the issue. Thus I suggest to add some type checks when `containerId` is provided with appropriate error message. This should result in a better developer experience.
kulla
force-pushed
the
z4uIW-kulla-2025-12-28-17-17
branch
from
124f902 to
4fad44b
Compare
ocavue
approved these changes
Feb 19, 2026
Contributor
Author
|
Ping @ocavue for merging 😄 |
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
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.
I run into an issue providing a container ID of an detached container. With the implementation before I got an
Uncaught TypeErrorsince there is no type check whethergetContainerId()returns a container so thatmap.get()cannot be called (seeloro-prosemirror/src/lib.ts
Lines 78 to 83 in b2bcee5
Thus I suggest to add some type checks when
containerIdis provided with appropriate error message. This should result in a better developer experience.Note: Due to #66 I haven't checked my code.