fix: keep leading no-op operations from consuming undo-step boundaries#2983
Draft
christianhg wants to merge 1 commit into
Draft
fix: keep leading no-op operations from consuming undo-step boundaries#2983christianhg wants to merge 1 commit into
christianhg wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: c7dbf1b 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 (04b7973) |
|---|---|---|
| Internal (raw) | 804.7 KB | +631 B, +0.1% |
| Internal (gzip) | 154.5 KB | +173 B, +0.1% |
| Bundled (raw) | 1.42 MB | +631 B, +0.0% |
| Bundled (gzip) | 318.8 KB | +169 B, +0.1% |
| Import time | 96ms | +0ms, +0.3% |
@portabletext/editor/behaviors
| Metric | Value | vs main (04b7973) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | +0ms, +0.1% |
@portabletext/editor/plugins
| Metric | Value | vs main (04b7973) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | +0ms, +1.9% |
@portabletext/editor/selectors
| Metric | Value | vs main (04b7973) |
|---|---|---|
| Internal (raw) | 81.8 KB | - |
| Internal (gzip) | 15.2 KB | - |
| Bundled (raw) | 77.6 KB | - |
| Bundled (gzip) | 14.1 KB | - |
| Import time | 8ms | +0ms, +0.9% |
@portabletext/editor/traversal
| Metric | Value | vs main (04b7973) |
|---|---|---|
| Internal (raw) | 28.1 KB | - |
| Internal (gzip) | 5.6 KB | - |
| Bundled (raw) | 28.1 KB | - |
| Bundled (gzip) | 5.5 KB | - |
| Import time | 6ms | -0ms, -1.1% |
@portabletext/editor/utils
| Metric | Value | vs main (04b7973) |
|---|---|---|
| Internal (raw) | 29.7 KB | - |
| Internal (gzip) | 6.2 KB | - |
| Bundled (raw) | 27.4 KB | - |
| Bundled (gzip) | 5.9 KB | - |
| Import time | 6ms | -0ms, -1.1% |
🗺️ . · ./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 (04b79737)
| Metric | Value | vs main (04b7973) |
|---|---|---|
| Internal (raw) | 53.8 KB | - |
| Internal (gzip) | 9.8 KB | - |
| Bundled (raw) | 348.9 KB | - |
| Bundled (gzip) | 96.3 KB | - |
| Import time | 38ms | -1ms, -2.9% |
🗺️ 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
fix-undo-step-boundaries
branch
from
July 20, 2026 16:26
56ea241 to
8d79883
Compare
The history subscriber's `isNoOp` early-return advanced `previousUndoStepId` before bailing. A no-op (an operation without an inverse, or a zero-length text operation) leading a new undo step then consumed the step boundary: the next history-affecting operation saw `currentUndoStepId === previousUndoStepId` and merged into the previous step, so a single undo reverted more than the last step's worth of changes. `previousUndoStepId` now only advances for operations that reach `createUndoSteps`, since it tracks the last operation that affected history. Relatedly, a step's pre-state selection was captured from the first history-affecting operation's `beforeSelection`. Operation implementations may park the selection on transient nodes (created and removed within the same step) before that operation applies, and undo then restored a selection that no longer resolves, dropping the cursor. The engine now snapshots its selection as `undoStepSelection` when `editor.undoStepId` is minted, and the history subscriber uses that snapshot as the step's pre-state selection when an operation opens a new step. Both defects are masked on `main` in the annotation and block-merge flows by an unconditionally applied whole-array `markDefs` set operation that reached `createUndoSteps` first, creating the boundary and capturing the selection before it moved. Emitting item-level `markDefs` operations removes that accidental mask, which is what surfaced both.
christianhg
force-pushed
the
fix-undo-step-boundaries
branch
from
July 21, 2026 08:30
8d79883 to
c7dbf1b
Compare
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 single undo can revert more than the most recent edit. The history subscriber's
isNoOpearly-return (operations without an inverse, zero-length text operations) advancedpreviousUndoStepIdbefore bailing, so a no-op leading a new undo step consumed the step boundary: the next history-affecting operation sawcurrentUndoStepId === previousUndoStepIdand merged into the previous step. One undo then reverted two edits' worth of changes.previousUndoStepIdnow only advances for operations that reachcreateUndoSteps, since its job is to track the last operation that affected history.This is reachable on
mainthrough public API:child.setreplacing a span's text leads with aremove.textof the old text, which is zero-length when the span was empty. The new scenario "Undoing achild.setthat replaces an empty span's text" pins it: two edits throughexecuteactions, one undo, only the second edit reverts. Onmainthe same test fails with both edits reverted. The producer isn't unique tochild.set:applySplitNodeemits an unguarded zero-lengthremove.textwhen splitting a span at its end, which the annotation and block-merge flows reach today, masked only because an unconditionally applied whole-arraymarkDefsset happens to hit the history subscriber first. #2982 removes that masking operation, which is how both defects surfaced; it stacks on this.The same masking hid a second defect: a step's pre-state selection was captured from the first history-affecting operation's
beforeSelection, but operation implementations can park the selection on transient nodes (created and removed within the same step) before that operation applies. Undo then restored a selection that no longer resolves and the cursor disappeared. The engine now snapshots its selection asundoStepSelectionat the momenteditor.undoStepIdis minted, and the history subscriber uses that snapshot as the pre-state selection whenever an operation opens a new step. This half is exercised without the mask by the custom-event undo scenarios in #2982; onmainno unmasked flow reaches it, so its pin lives there.Net behavior unchanged for flows whose steps begin with a history-affecting operation; the full unit and browser suites pass without modification.