Skip to content

fix: emit reliable patches after clearing the editor - #3236

Merged
christianhg merged 2 commits into
mainfrom
fix-empty-editor-rematerialization
Sep 8, 2026
Merged

fix: emit reliable patches after clearing the editor#3236
christianhg merged 2 commits into
mainfrom
fix-empty-editor-rematerialization

Conversation

@christianhg

@christianhg christianhg commented Sep 7, 2026

Copy link
Copy Markdown
Member

Clearing a Portable Text field and typing into it again could silently lose the new text, or crash the consumer applying the emitted patches (Cannot apply deep operations on primitive values): the retype flush emitted only diffMatchPatches against a field the same patch stream had just unset([]). It reproduces with any host that mirrors mutation.value back into update value, which is the documented wiring (the playground uses it), and it is live in the playground on main.

The editor tells a synced-in pristine block (persisted content) apart from its own local placeholder by remembering the last synced value. Two paths let the placeholder itself become that evidence. First, the sync machine recorded every completed sync, including no-op syncs whose value the engine already embodied, so the mirror's echo after a single-flush clear recorded the placeholder directly. Second, and surviving any recording guard: when a clear spans several flushes (character-by-character backspace), the mirrored echoes lag the ongoing edits, so each stale echo differs from the engine by the time it syncs, writes as a genuine remote change, and the last one legitimately records the cleared state.

Two mechanisms, one per path. Recording now happens only when a sync actually wrote into the engine, since an echo of the editor's own state asserts nothing about persistence. And patch generation tracks its own field-level statement: after it emits the became-empty unset([]), the next content-producing edit always rebuilds the field, regardless of what lagging syncs recorded in between. Rebuilding over a recorded value equal to the placeholder also clears the recording (the rebuild proves it was a stale echo), so a repeated clear unsets the field again instead of leaving an empty block behind. The emitted stream stays self-consistent: destroy, then rebuild, never text diffs into the void.

One narrow trade rides along, disclosed in the changeset: a genuine empty block arriving through update value in the window between the editor's own unset and its next keystroke is now rebuilt (re-inserting a block the document already holds) where value sync previously suppressed correctly; the same window reached through remote patches misbehaved identically before this change. A stale echo and a genuine pristine block are indistinguishable by value in that window; distinguishing them takes provenance metadata, which is follow-up work. Both regression scenarios from the persisted-content fix this builds on remain pinned and green; the two new scenarios were proven red five-for-five without each mechanism and stable across twenty runs with them.


Note

Medium Risk
Changes patch emission and value-sync bookkeeping on the critical clear→retype path; behavior is narrow but affects how hosts apply local patches.

Overview
Fixes a bug where clearing the editor and typing again could emit only diffMatchPatch patches against a field the same stream had just removed with unset([]), so consumers lost the new text or threw when applying patches. This showed up with the common host pattern that mirrors mutation.value back into update value.

Patch generation now tracks valueUnsetEmitted after a became-empty unset([]). The next local edit that produces patches always re-materializes the field (setIfMissing, block insert, then text diffs), even if a lagging mirrored sync made the placeholder look like persisted content. Rebuilding over a recorded value equal to that placeholder clears lastSyncedValue so a second clear still emits unset([]).

Value sync only updates lastSyncedValue when a completed sync actually wrote into the engine (done syncing carries a changed flag), so no-op echoes of the editor’s own state are not treated as host persistence claims.

Regression coverage adds scenarios for single-flush and character-by-character clears with mirrored values.

Reviewed by Cursor Bugbot for commit 946c2ca. Bugbot is set up for automated code reviews on this repo. Configure here.

@changeset-bot

changeset-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 946c2ca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages
Name Type
@portabletext/editor Patch
@portabletext/plugin-character-pair-decorator Patch
@portabletext/plugin-dnd Patch
@portabletext/plugin-emoji-picker Patch
@portabletext/plugin-input-rule Patch
@portabletext/plugin-list-index Patch
@portabletext/plugin-markdown-shortcuts Patch
@portabletext/plugin-one-line Patch
@portabletext/plugin-paste-link Patch
@portabletext/plugin-sdk-value Patch
@portabletext/plugin-table Patch
@portabletext/plugin-typeahead-picker Patch
@portabletext/plugin-typography Patch
@portabletext/toolbar Patch

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

@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
portable-text-editor-documentation Ready Ready Preview Sep 8, 2026 6:29am UTC
portable-text-example-basic Building Building Preview Sep 8, 2026 6:29am UTC
portable-text-playground Ready Ready Preview Sep 8, 2026 6:29am UTC

Request Review

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Bundle Stats

✅ No significant changes.

All scenario measurements (7)

🗺️ @portabletext/editor / @portabletext/editor · @portabletext/editor / @portabletext/editor/behaviors · @portabletext/editor / @portabletext/editor/plugins · @portabletext/editor / @portabletext/editor/selectors · @portabletext/editor / @portabletext/editor/traversal · @portabletext/editor / @portabletext/editor/utils · @portabletext/markdown / @portabletext/markdown · Artifacts

Scenario Kind Bundle (raw / gzip) Gzip change Import time Import change
⚪ @portabletext/editor / @portabletext/editor export 1.09 MB / 254.4 KB +68 B, +0.0% 67 ms -0 ms, -0.3%
⚪ @portabletext/editor / @portabletext/editor/behaviors export 4.0 KB / 1.4 KB None 2 ms -0 ms, -2.6%
⚪ @portabletext/editor / @portabletext/editor/plugins export 5.1 KB / 1.8 KB None 7 ms -0 ms, -1.0%
⚪ @portabletext/editor / @portabletext/editor/selectors export 94.8 KB / 21.7 KB None 8 ms +0 ms, +0.7%
⚪ @portabletext/editor / @portabletext/editor/traversal export 42.8 KB / 11.2 KB None 6 ms +0 ms, +0.6%
⚪ @portabletext/editor / @portabletext/editor/utils export 34.0 KB / 9.1 KB None 6 ms +0 ms, +2.3%
⚪ @portabletext/markdown / @portabletext/markdown export 292.1 KB / 86.0 KB None 40 ms -1 ms, -1.7%

Significant means at least 1.0 KB and 1% gzip, or at least 5 ms and 10% import time.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 677a27b. Configure here.

Comment thread packages/editor/src/editor/sync-machine.ts
@christianhg
christianhg force-pushed the fix-empty-editor-rematerialization branch from 677a27b to ec1480a Compare September 7, 2026 12:07
@christianhg
christianhg force-pushed the fix-empty-editor-rematerialization branch from ec1480a to c80781e Compare September 8, 2026 06:23
@christianhg christianhg changed the title fix: only record a synced value as persisted when the sync wrote it fix: don't lose text typed right after clearing the editor Sep 8, 2026
@christianhg
christianhg force-pushed the fix-empty-editor-rematerialization branch from c80781e to 384ca70 Compare September 8, 2026 06:24
@christianhg
christianhg force-pushed the fix-empty-editor-rematerialization branch from 384ca70 to b772de3 Compare September 8, 2026 06:24
@christianhg
christianhg force-pushed the fix-empty-editor-rematerialization branch from b772de3 to c004dd0 Compare September 8, 2026 06:25
@christianhg christianhg changed the title fix: don't lose text typed right after clearing the editor fix: emit reliable patches after clearing the editor Sep 8, 2026
@christianhg
christianhg force-pushed the fix-empty-editor-rematerialization branch from c004dd0 to 644bee2 Compare September 8, 2026 06:26
@christianhg
christianhg force-pushed the fix-empty-editor-rematerialization branch from 644bee2 to b800c69 Compare September 8, 2026 06:27
…ring host

A host following the documented mirror pattern (send `mutation.value`
back as `update value`) echoes the editor's own placeholder-bearing
engine value after a clear. The retype must still emit
`setIfMissing([], [])` and the block `insert` before its
`diffMatchPatch`es: the clear flush ended with `unset([])`, so the
patch stream's document has no field to receive naked text diffs.

Two scenarios, two poisoning paths. A single-flush clear (word delete)
echoes a value equal to the engine, which only a no-op sync recording
turns into false persistence evidence. A character-by-character clear
spreads across flushes, so the echoes lag the ongoing edits: each
stale echo differs from the engine by the time it syncs, writes as a
genuine remote change, and the last one records the editor's cleared
state as synced content. The second scenario waits for that cascade
to settle (a remote-origin operation observed, engine back at the
placeholder) before retyping, so the poisoned recording is
deterministically in place.

Red before the fix in both scenarios: the placeholder passes as
persisted content, re-materialization is suppressed, and the retype
flush carries only `diffMatchPatch`es that no patch-applying store
can apply.
`lastSyncedValue` exists to tell a synced-in pristine block (persisted
content) apart from the editor's local placeholder. Two paths let the
placeholder itself get recorded as that evidence when a host mirrors
`mutation.value` back as `update value`. The sync machine's
`record synced value on engine` recorded every `done syncing` value,
including no-op syncs whose value the engine already embodied, so a
mirror echo after a single-flush clear recorded the placeholder
directly. And when a clear spreads across flushes, the echoes lag the
edits: each stale echo differs from the engine by sync time and writes
as a genuine remote change, so even a correctly guarded recording ends
up holding the editor's cleared state. Either way, the placeholder then
compared equal to `lastSyncedValue` in `subscriber.patch-generation.ts`,
the empty-editor transitions were suppressed, and the retype after a
clear emitted naked `diffMatchPatch`es against a field its own stream
had just `unset([])`.

Two mechanisms, one per path. `updateValue` already computes
`isChanged`; it rides on `done syncing` as `changed`, and
`lastSyncedValue` is recorded only when the sync actually wrote, since
an echo of the editor's own state asserts nothing about persistence.
And patch generation now tracks its own field-level statement on the
engine (`valueUnsetEmitted`): set when the became-empty `unset([])`
is emitted, cleared when the rebuild is emitted, and while set,
`editorWasEmpty` ignores the `lastSyncedValue` comparison entirely.
The emitted stream stays self-consistent regardless of what lagging
syncs recorded in between: after destroying the field it always
rebuilds it before targeting it again. Rebuilding over a recorded
value equal to the placeholder also clears the recording: the rebuild
proves it was a stale echo, and keeping it would make the next
became-empty transition skip its `unset([])`.

Recording now skips every no-op sync, not only mirror echoes. Emitted
patches only change after a became-empty `unset([])`: the next
content-producing edit re-materializes the field again. Genuinely
synced-in empty blocks still suppress re-materialization (no self-emitted
unset precedes them), pinned by the existing pristine-block scenarios.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant