fix: re-materialize the field before patches emitted while it is unset - #3253
Merged
Conversation
…placement A behavior can raise a root `unset` followed by `insert.block` in one action set, taking the engine through a zero-block state between the operations. The emitted patch stream must stay applicable to a patch-applying store throughout: after `unset([])` destroys the field, `setIfMissing([], [])` must precede the `insert`, or the store throws `Cannot apply deep operations on primitive values`. Four scenarios: the plain replace-all (root `unset` then `insert.block`), the same raised twice (`unset`, `unset`, `insert`: no rebuild between the unsets, one before the insert), replace-all raised while the editor is the empty placeholder (the placeholder rebuild triple precedes the `unset`), and a `history.redo` replay of the replace-all group. Each scenario closes by applying the collected patches with `applyAll` and asserting equality with the editor's final value. Red before the fix: the streams carried a naked `insert` after the `unset([])` (and the double-unset order emitted a pointless `setIfMissing` between the unsets), so `applyAll` threw. The redo scenario is coverage for the replay path, green on both sides.
`valueUnsetEmitted` tracked the destroyed-field state from two ad-hoc write sites, both keyed to the length-1 placeholder transition (`editorWasEmpty`/`editorIsEmpty` in `subscriber.patch-generation.ts`). A behavior raising a root `unset` mid action set takes the engine through a genuine zero-block state those predicates never match: the `unset([])` went out without arming the flag, the following `insert` went out naked, and a patch-applying store threw `Cannot apply deep operations on primitive values`. The flag is now derived from the emitted stream itself. After an event's patch list is fully composed, a single walk sets the flag on a root `unset` and clears it on a root `setIfMissing` or `set`, replacing every ad-hoc write. Patches generated while the previous value has zero blocks and the flag is armed get `setIfMissing([], [])` prepended, unless the first patch is itself a root unset (a second destroy needs no rebuild) or already root-materializing. The walk placement makes the flag truthful in orders the ad-hoc writes got wrong: double root unset keeps it armed between the unsets, and a replace-all raised from the empty placeholder ends armed even though the event started with the placeholder rebuild. One narrow behavioral fix rides along, disclosed in the changeset: behavior-raised root unsets now arm the same stale-echo re-materialization guard that keyboard clearing already had, so a retype after such a clear rebuilds the field under a value-mirroring host. Emitted patches are otherwise unchanged for every path that does not pass through a destroyed field.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: db5b02a 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.
A behavior that replaces the whole value in one action set (raise a root
unset, theninsert.block) emitted a patch stream no patch-applying store could apply: theinserttargeted a field the precedingunset([])had just removed, and applying it throwsCannot apply deep operations on primitive values. The editor itself ended in the intended state; only the emitted stream was broken.The became-empty bookkeeping (
valueUnsetEmitted) was written from two ad-hoc sites, both keyed to the length-1 placeholder transition. A grouped action set takes the engine through a genuine zero-block state that shape never matches, so theunset([])went out without arming the flag and theinsertwent out naked. The flag is now derived from the emitted stream itself: after an event's patches are composed, one walk arms it on a rootunsetand disarms it on a rootsetIfMissing/set, and patches emitted while the field is destroyed getsetIfMissing([], [])prepended. Deriving instead of hand-writing also fixes the orders the ad-hoc writes got wrong, each pinned by its own scenario: a double root unset keeps the flag armed between the unsets, and a replace-all raised from the empty placeholder ends armed even though its event began with the placeholder rebuild. A redo-replay scenario covers the history path.One narrow fix rides along, disclosed in the changeset: behavior-raised root unsets now arm the same re-materialization guard that keyboard clearing already had, so a retype after such a clear rebuilds the field under a value-mirroring host. Emitted patches are unchanged for every path that does not pass through a destroyed field.
Note
Medium Risk
Changes core patch emission and empty-field bookkeeping; incorrect logic could alter patch streams for clear/replace flows, though scope is narrow and heavily tested.
Overview
Fixes broken patch streams when behaviors replace the whole document in one action set (root
unsettheninsert.block). Stores could not apply the emittedinsertafterunset([]), causing Cannot apply deep operations on primitive values; the editor state was correct, only the outbound patches were wrong.Patch generation now tracks whether the root field is “destroyed” via
valueUnsetEmitted, derived by scanning each batch of emitted patches (rootunsetarms it; rootsetIfMissing/setdisarms it) instead of toggling the flag at two placeholder-transition sites. When the stream is still unset and the next patches are not themselves destroy/rebuild, it prependssetIfMissing([], [])so downstream inserts apply cleanly. Double root unsets, replace-all from an empty placeholder, and history redo are covered by new tests.A related fix: clearing via a behavior-raised root
unsetnow matches keyboard clear—retyping under a value-mirroring host re-materializes the field before content patches.Reviewed by Cursor Bugbot for commit db5b02a. Bugbot is set up for automated code reviews on this repo. Configure here.