fix: skip cosmetic normalization while applying remote patches#3001
Merged
christianhg merged 2 commits intoJul 22, 2026
Conversation
🦋 Changeset detectedLatest commit: 3cd8679 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 —
|
| Metric | Value | vs main (11d72d0) |
|---|---|---|
| Internal (raw) | 806.9 KB | +313 B, +0.0% |
| Internal (gzip) | 155.0 KB | +108 B, +0.1% |
| Bundled (raw) | 1.42 MB | +313 B, +0.0% |
| Bundled (gzip) | 319.3 KB | +106 B, +0.0% |
| Import time | 97ms | -4ms, -4.1% |
@portabletext/editor/behaviors
| Metric | Value | vs main (11d72d0) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | -0ms, -3.3% |
@portabletext/editor/plugins
| Metric | Value | vs main (11d72d0) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | -0ms, -1.2% |
@portabletext/editor/selectors
| Metric | Value | vs main (11d72d0) |
|---|---|---|
| Internal (raw) | 82.7 KB | - |
| Internal (gzip) | 15.4 KB | - |
| Bundled (raw) | 78.4 KB | - |
| Bundled (gzip) | 14.3 KB | - |
| Import time | 8ms | -0ms, -1.1% |
@portabletext/editor/traversal
| Metric | Value | vs main (11d72d0) |
|---|---|---|
| Internal (raw) | 28.1 KB | - |
| Internal (gzip) | 5.6 KB | - |
| Bundled (raw) | 28.1 KB | - |
| Bundled (gzip) | 5.5 KB | - |
| Import time | 6ms | -0ms, -2.2% |
@portabletext/editor/utils
| Metric | Value | vs main (11d72d0) |
|---|---|---|
| Internal (raw) | 30.6 KB | - |
| Internal (gzip) | 6.4 KB | - |
| Bundled (raw) | 28.2 KB | - |
| Bundled (gzip) | 6.1 KB | - |
| Import time | 6ms | -0ms, -0.9% |
🗺️ . · ./behaviors · ./plugins · ./selectors · ./traversal · ./utils · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @portabletext/markdown
Compared against main (11d72d02)
| Metric | Value | vs main (11d72d0) |
|---|---|---|
| Internal (raw) | 53.8 KB | - |
| Internal (gzip) | 9.8 KB | - |
| Bundled (raw) | 348.9 KB | - |
| Bundled (gzip) | 96.3 KB | - |
| Import time | 38ms | -0ms, -0.3% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
christianhg
force-pushed
the
skip-cosmetic-normalization-during-remote-patches
branch
from
July 22, 2026 08:17
91e81d2 to
629fed1
Compare
christianhg
marked this pull request as ready for review
July 22, 2026 08:17
christianhg
force-pushed
the
skip-cosmetic-normalization-during-remote-patches
branch
from
July 22, 2026 08:30
629fed1 to
f7f511a
Compare
Red at this commit by design: the suite reproduces the corruption the
next commit fixes. A collaborator's un-bold delivered across two
listener events makes the receiving editor merge the transient
same-mark adjacency and emit mirror-image merge patches; applied at
the document, their `diffMatchPatch`es re-insert already-merged text
("foobarbaz" becomes "foobarbarbaz", the duplicated-tail signature
from the field report). The suite also pins the mechanism: remote
structure kept as-is, nothing emitted, re-canonicalization on the next
local edit, and `update value` still canonicalizing (green before and
after the fix).
Normalization rules fall into two classes. Repairs fix structure the engine cannot represent (missing `_key`/`_type`, duplicate keys, missing required fields, empty blocks, unbracketed inline objects). Cosmetic rules canonicalize structure that is already valid Portable Text: merging adjacent same-mark spans and dropping empty sibling spans. Running the cosmetic class as remote-patch fallout is wrong in both directions. Emitted text patches are `diffMatchPatch` against a keyed span's text, so the wire structure is the shared base between editor and store. A receiver that merges a collaborator's spans either pushes the merge and becomes a competing writer on the originator's spans (formatting lost, tail text duplicated under concurrent edit), or keeps it local and forks the patch base so its own next edit in that span misapplies at the store. `normalizeNode` now skips the two cosmetic sites (the same-marks merge and the empty/loose-equal merge arm of the children loop) while `editor.isProcessingRemoteChanges` is set. The flag already has exactly the right scope: `setupRemotePatches` flushes normalization inside `withRemoteChanges`, while the sync machine's `update value` flush runs outside it (its `withoutNormalizing` is outermost), so self-solving still canonicalizes loaded documents and local edits still merge and push as the author's own mutation. Net behavior change is confined to the remote-patch path: adjacent same-mark spans and empty sibling spans arriving from a collaborator are kept as-is (they render identically) and re-canonicalize on the block's next local edit. Repairs still run and emit during remote apply.
christianhg
force-pushed
the
skip-cosmetic-normalization-during-remote-patches
branch
from
July 22, 2026 08:40
f7f511a to
3cd8679
Compare
christianhg
deleted the
skip-cosmetic-normalization-during-remote-patches
branch
July 22, 2026 08:45
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.
Two people editing the same block concurrently could watch formatting revert and the tail of the block duplicate: the receiving editor's normalizer merged adjacent same-mark spans created by composing remote patches with its own state, then pushed those merges back at the originator (diagnosed in #3000).
normalizeNode's rules now split into two classes:_key/_type, duplicate keys, missing required fields (markDefs,style,text,marks), empty blocks, unbracketed inline objects, orphaned annotation marks. These run on every path, unchanged.Cosmetic rules still run for local edits (the merge is pushed as the author's own mutation) and on
update value(self-solving keeps healing loaded documents). They are skipped in exactly one case: normalization triggered by applying remote patches, detected via the existingeditor.isProcessingRemoteChangesflag. A collaborator's structure is kept as-is, renders identically, and is canonicalized by whoever edits that block next.Skipping the merge, rather than suppressing its push (#3000's approach), is deliberate: emitted text patches are
diffMatchPatchagainst a keyed span's text, so span structure is the shared base between editor and store. Merging a collaborator's spans is wrong whether the merge is pushed (competing writers, the duplication above) or kept local (the receiver's structure forks from the store and its next edit in that span misapplies).Pinned in
remote-patches.cosmetic-normalization.test.tsx, red without the gate, including the no-emission asserts and anupdate valuepin that also guards the flag's scope (its flush runs outsidewithRemoteChanges; a wrapper-nesting refactor in the sync machine would go red here instead of silently disabling self-solving). Two pins inevent.patches.test.tsxasserted merge-on-remote-apply and are re-pinned; their actual subjects (container traversal,_typerepair) are unchanged. Full browser and unit suites green. With this, #3000's push-boundary gate has no echo left to drop.The first commit carries the reproduction and is red by design; the fix commit turns it green. At the test commit, the end-to-end scenario (a collaborator's un-bold delivered across two listener events, the receiver echoing its mirror-image merge) fails with the field report's exact signature:
The duplication needs no concurrent typing: the collaborator's own merge
diffMatchPatches ("insertbarafterfoo") re-apply against the receiver's already-merged text, and the receiver's echo does the same to the document.git checkout HEAD~1to witness it red.