fix: dedupe identical same-key markDefs on every block merge - #3195
Draft
christianhg wants to merge 2 commits into
Draft
fix: dedupe identical same-key markDefs on every block merge#3195christianhg wants to merge 2 commits into
christianhg wants to merge 2 commits into
Conversation
…g text block fragments `adjustFragmentKeys` renamed a fragment markDef whenever its `_key` existed on the destination block, on key membership alone. A def that is byte-identical to the destination's is the same annotation, so the rename churned identity for nothing: consumers that track annotations by `_key` (comments, decorations) lost the annotation they were following, and the caller's plain markDefs append then stored the same content twice under two keys. Colliding defs are now compared with `isDeepEqual`: an identical def keeps its key, is excluded from the defs appended into the destination, and its spans' `marks` stay untouched, so they resolve to the destination's copy. Same-key defs with different content rename exactly as before, pinned by a contrast test that fails when the equality branch is forced. One rider: `isDeepEqual` gains an explicit `boolean` return type. The inferred type predicate narrowed the non-equal branch of same-type comparisons to `never`, which broke compilation at the new call site.
`planMergeKeyRenames` carried a required `dedupeEqualMarkDefs` flag: the range-delete merge deduped identical same-key defs, but the collapsed backspace/forward-delete merges had to keep renaming them, because their `renamedBlock` feeds `insert.block`, where `parseBlock` parses the block standalone and strips any mark that does not resolve in the block's own `markDefs`: a deduped def's spans would have lost the annotation entirely. With `insert.block`'s fragment adjustment now treating identical defs as non-conflicts, that constraint is gone and the flag deletes. The planner always keeps an identical same-key def unrenamed and inside `renamedBlock.markDefs` (the block must carry the def so `parseBlock` resolves its marks) and reports it in `dedupedMarkDefKeys`; `mergeBlock` skips appending those keys. Net behavior change: the collapsed merges now dedupe. Splitting an annotated block and merging it back rejoins the annotation into one def under its original key, where it previously duplicated the def and re-keyed half the spans; the `annotations-across-blocks` gherkin scenario that pinned the old outcome under a 'possible wrong behaviour' warning now pins the rejoin, and the `unique-sibling-keys` pin of the same shape follows. Undo of a deduped merge restores both original blocks byte-identically, pinned by an undo-redo-undo round-trip. The range-delete path's behavior is unchanged.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: cad0f5e 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 |
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. |
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.
An annotation's
_keyis its identity: comments and decorations follow it, and receivers pair patches through it. Two paths treated a byte-identical markDef under the same_keyas a conflict and renamed it, churning that identity for content that is the same annotation:insert.block's fragment adjustment (which also duplicated the def through its plain markDefs append), and, forced by it, the collapsed backspace/forward-delete merges. The range-delete merge already deduped (#3187), shipping the asymmetry behind adedupeEqualMarkDefsflag.The first commit fixes the root:
adjustFragmentKeyscompares colliding defs withisDeepEqual; an identical def keeps its key, is excluded from the appended defs, and its spans' marks resolve to the destination's copy. Different-content defs rename as before, pinned by a contrast test that fails when the equality branch is forced. The second commit deletes the flag: the planner always keeps an identical def unrenamed and inside the merging block (it must stay there, sinceinsert.blockparses blocks standalone and strips marks that don't resolve in the block's ownmarkDefs), reporting it somergeBlockskips the append.The user-visible delta beyond the rename churn: splitting an annotated block and merging it back now rejoins the annotation into one def under its original key. The gherkin scenario that pinned the old duplicate-def outcome under a "possible wrong behaviour" warning now pins the rejoin. Undo of a deduped merge restores both original blocks byte-identically, pinned by an undo-redo-undo round-trip; wire-catalogue fixtures are untouched.