fix: prevent crash when applying Yjs update with invalid Slate path#430
Open
juliankrispel wants to merge 4 commits into
Open
fix: prevent crash when applying Yjs update with invalid Slate path#430juliankrispel wants to merge 4 commits into
juliankrispel wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: 853abd4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
- Update .nvmrc from lts/gallium (Node 16 EOL) to lts/iron (Node 20 LTS) - Upgrade actions/checkout from v2 to v4 - Upgrade actions/setup-node from v2 to v4 - Upgrade docker/login-action from v1 to v3 - Upgrade docker/metadata-action from v3 to v5 - Upgrade docker/build-push-action from v2 to v5 Node 16 reached End of Life in September 2023, causing CI failures due to GitHub Actions cache service issues with EOL Node versions.
1 task
When applying a remote Yjs update to the Slate document, the code can call Node.get(editor, path) with a path that no longer exists (e.g. after paste/duplication or path skew). This causes Slate to throw "Cannot find a descendant at path", crashing the app. This change adds defensive handling in the Yjs→Slate apply path: - Catch sync-skew errors (path not found, yOffset out of bounds, etc.) - Re-sync the Slate editor from Yjs when an error is detected - Preserve user selection via Yjs relative positions during re-sync - Throttle re-syncs to max once per second to avoid performance issues - Return null instead of throwing in absolutePositionToSlatePoint Scope: Only the Yjs→Slate apply path; no API changes.
juliankrispel
force-pushed
the
fix/defensive-path-handling
branch
from
60ec988 to
744d1a2
Compare
|
@juliankrispel We are also using slate with yjs and see some unexpected behaviour. Do you know if smeone still maintains this library? |
Owner
|
Nope - not working on it anymore! Happy to make you maintainer @juliankrispel if interested |
|
I would be also interested to contribute @BitPhinix. We are using it within plate and there are some things that need improvement |
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
3 participants
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.
Summary
When applying a remote Yjs update to the Slate document, the code can call
Node.get(editor, path)with a path that no longer exists (e.g. after paste/duplication or path skew). This causes Slate to throw "Cannot find a descendant at path", crashing the app. This is a reoccuring issue with this library unfortunately.This PR adds defensive handling in the Yjs→Slate apply path:
applyYjsEventsconnect())nullinstead of throwing inabsolutePositionToSlatePointfor cursor safetyScope
packages/coreChanges
packages/core/src/utils/errors.ts- Helper to detect sync-skew errorspackages/core/src/applyToSlate/index.ts- Try/catch with re-sync, selection preservation, throttle guardpackages/core/src/utils/position.ts- Defensive handling inabsolutePositionToSlatePointTest plan