fix: rename colliding keys before a text block merge deletes the merging block - #3182
Merged
Conversation
🦋 Changeset detectedLatest commit: c9a16d7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Bundle Stats✅ No significant changes. All scenario measurements (7)🗺️
Significant means at least 1.0 KB and 1% gzip, or at least 5 ms and 10% import time. |
christianhg
marked this pull request as ready for review
August 26, 2026 11:03
christianhg
force-pushed
the
merge-rekey-main
branch
from
August 26, 2026 11:12
5f63479 to
b373361
Compare
christianhg
changed the title
…key` changes The `rekey` step rewrote any point path segment bearing the old key, path-blind. Keys are only unique among siblings, so a node elsewhere in the document can legally carry the same key, and a rename would drag positions on that twin to the renamed node's location. The step now carries the renamed node's container path, and the mapper rewrites only the segment at that exact location. `transform-point`'s own `rekey` construction for local `_key` sets supplies the same path. Pinned by a unit test placing a same-valued key at a different depth and asserting it stays put.
…ing block Backspace at the start of a text block and forward delete at the end of the block before it both merge by raising `delete.block` + `insert.block`, and `adjustFragmentKeys` silently minted fresh keys for any child or markDef colliding with the destination block, so the wire read destroy-and-create and receivers could connect nothing through the merge. Both merge behaviors now plan the renames first: `set` patches on keyed `_key` paths (markDef renames rewrite the referencing `marks`), raised while the merging block is still addressable, and the captured node passed to `insert.block` carries the renamed keys, so no collision handling fires downstream. The wire becomes rename, unset, reinsert under final keys. Rename-first is load-bearing: renaming after insertion would put twin keys among siblings. Non-colliding merges emit byte-identical patches to before. Pinned by three wire-catalogue rows (`block-merge-duplicate-keys`, `-forward`, `-markdefs`) and two-editor browser tests covering both gestures' emission shapes and the markDef rename rewrites.
The auto-generated inverse for a `set` on a node's own `_key` addressed the node by its pre-rename keyed path, which stops resolving the moment the rename applies, so undo silently dropped the rename while reverting everything else in the step. Both inverse constructions (`setNodeProperties` and the engine's `set` apply path) re-target the post-rename key. Redo is unaffected: it replays the original operations, whose pre-rename paths resolve again after undo. Pinned by a merge-undo-redo-undo round-trip over the colliding-keys merge, asserting the exact full value, every `_key` included, at each stop.
christianhg
force-pushed
the
merge-rekey-main
branch
from
August 26, 2026 11:16
b373361 to
c9a16d7
Compare
Merged
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
1 participant
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.
Merging a text block into its neighbor (backspace at its start, or forward delete at the end of the block before it) re-keys any child span or annotation whose
_keycollides with one in the destination block. Today those fresh keys are minted silently before insertion, so the wire reads "block deleted, unrelated spans created": receivers can connect nothing through the merge, and anything tracking those nodes (carets, comments, decorations) loses them.The fix makes both merge gestures say what they did. Before deleting the merging block, the merge behaviors rename the colliding keys in place,
setpatches on keyed_keypaths, rewritingmarksreferences when a markDef is renamed, and insert the children under their final keys. Rename-first matters: renaming after insertion would put twin keys among siblings. Non-colliding merges emit byte-identical patches to before.Two engine fixes ride along in their own commits, both latent bugs this work surfaced and both reachable today without the merge change:
rekeypoint transform rewrote positions by key value alone, so renaming a node dragged carets and tracked ranges sitting on a same-keyed twin elsewhere in the document. It now rewrites only at the renamed node's own path._keyrename silently dropped the rename: the auto-generated inverse addressed the node by a path that stopped resolving once the rename applied. Inverses now target the post-rename key, pinned by a merge-undo-redo-undo round-trip.Receiver-side caret following through this new wire shape (the transaction interpreter recognizing renamed, reinserted children) lands separately once #3154 merges; this PR is emission and engine correctness only.
Note
Medium Risk
Touches core delete/merge behaviors, patch emission order, selection transformation, and undo inverses—high user impact for editing and collaboration, but changes are targeted with broad test and wire-catalogue coverage.
Overview
Block merges (backspace at block start or forward delete at the prior block’s end) now rename colliding
_keys on the merging block before it is removed and re-inserted. Collisions on child spans andmarkDefsare handled via explicitset/child.setactions (including remappingmarkswhen an annotation key changes), so emitted patches describe rename then move instead of silent re-minting that looked like destroy-and-create on the wire. Non-colliding merges are unchanged.Selection and point mapping for
_keyrenames is path-scoped:rekeysteps carry the parent path and only rewrite the renamed node’s segment at that depth, so carets on another node that shares the same key (legal across blocks) are not pulled along.Undo of a
_keyrename now builds inverseset/unsetpaths against the new key, so reverting a merge (or any rename in the same undo batch) restores the original keys instead of leaving nodes stuck under renamed keys.Reviewed by Cursor Bugbot for commit c9a16d7. Bugbot is set up for automated code reviews on this repo. Configure here.