Repository navigation
fix(studio): undo takes back the last edit, even right after a resize or a nudge - #5048
Merged
Merged
Conversation
Edit accuracy: accurate 2040 (base branch 2040), smooth 1548 of thoseThe gate passes. Quarantined, measured but not gated (0) |
miguel-heygen
force-pushed
the
fix/studio-undo-order-pending-saves
branch
from
October 5, 2026 09:13
a253a4a to
38beaf0
Compare
miguel-heygen
force-pushed
the
fix/studio-undo-order-pending-saves
branch
from
October 5, 2026 15:06
2431f74 to
191e331
Compare
miguel-heygen
marked this pull request as ready for review
October 5, 2026 15:58
terencecho
approved these changes
Oct 5, 2026
terencecho
left a comment
Contributor
There was a problem hiding this comment.
No blocker at 191e331c34716a390d6d53d2ec842bcdb32efd28. Approving; the CI jobs were all green when I stamped (66 pass, 13 skipping).
What I checked (local, NODE_ENV=test)
- Changed files, three runs each, all green:
studioPendingEdits.test28,useDomEditNudge.test15,useDomGeometryCommits.test18,gsapSoftReload.test55,gsapUndoRestore.test40. Neighbours (one file at a time) pass:useEditHistoryActions13 + paint 10,useTrackPendingTimelineEdit3,useGsapScriptCommits37,DomEditContext2,dragUndoPaint7,gsapRuntimePatch20,gsapResizeElementSize3,groupPlan6,useTimelineDeleteOps10,timelineClipDragCommit60,usePersistentEditHistory24,usePreviewPersistence.history7,useRazorSplit.history5.tsc --noEmitinpackages/studiois clean. - Mutations (16, each reverted), 12 caught:
- Caught: per-element
box-sizekey back; no flush intrackedStudioEdit; no flush inbeginStudioPendingEdit; panel edits never wait; gestureadoptnever waits; internal saves allowed to wait (drops theadoptingguard, hangs/fails 10 tests); flush while adopting; never clearing the flushed-saves chain; norecordLiveSet; no live-set targets; no standalonegsap.settargets; noforgetLiveSets. - Survived: see non-blocking 1 to 3.
- Caught: per-element
- Call-site census:
trackedStudioEdit(has five production callers (DomEditContext,App.tsxz-reorder/delete,useGsapScriptCommitsx2,useTrackPendingTimelineEdit); onlyDomEditContextpassesafterOlderSaves, so Studio's internal commit paths still never wait. Edits that run inside a gesture commit are covered by theadoptingflag. - The
box-sizecoalesce key is now unique per commit, so a resize and the next size edit cannot merge; the legacy per-step undo for W/H scrubbing is the trade-off the PR names. - Overlap with #5045: it also edits
studioPendingEdits.tsandgsapUndoRestore.test.ts; a merge of the two heads is conflict-free (git merge-tree). - No secrets in the diff.
Non-blocking
DomEditContext.tsxpassing{ afterOlderSaves: true }is the production hook-up of the wait, and removing it keeps every test green (useDomEditNudge.testcallstrackedStudioEditwith the option directly). A smallDomEditContexttest (nudge burst pending, then a wrapped action) would pin it.- Dropping the previous chain entry (
Promise.allSettled([flushedSavesStillWriting, ...])) stays green: a second flush while the first save is still writing is not pinned to wait for it. coalesceMs: Number.POSITIVE_INFINITYinuseDomGeometryCommitsis redundant with the unique key (dropping it stays green); harmless.- A deferred edit returns a Promise cast as
R. The wrapped actions I looked at (handleGroupSelection,handleSetArcPath,handleUnroll) return void and are not awaited, so nothing breaks today, but an edit that throws synchronously inside the deferred path now rejects an unhandled promise instead of throwing. - I did not run the edit-accuracy browser bench (the 20-step undo chain, the 150 ms / 69 ms timings); those numbers are the author's.
Reviewed on 191e331c34716a390d6d53d2ec842bcdb32efd28. This is a review verdict, not authorization to merge.
— Review by tai (pr-review)
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
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.
What
Three cases where undo took back the wrong thing, or showed it wrong, are fixed. Each Cmd+Z now takes back exactly the last edit you made.
Why
Resize, then W. For an element without an animation, a canvas resize and a W or H edit in the Design panel save through the same commit (
handleDomBoxSizeCommit). Both recorded their history entry under one undo key per element,box-size:<element>, with the default 300 ms merge window. A size edit that reached history within 300 ms of the resize was added to the resize's entry.Nudge, then panel edit. A burst of arrow-key nudges saves once, after a short pause (
CANVAS_NUDGE_COMMIT_DEBOUNCE_MS). A pointer gesture and undo both made a still-waiting burst save first. A Design-panel edit did not, so its save, and its history entry, landed ahead of the nudge's.How
Resize, then W. Each box-size commit gets its own undo key with no time limit, the way a plain rotate already does (
plainRotation.ts, #4802). A commit's own writes stay one step, and a later edit never merges into it. Gestures that pass their own key (GSAP resizes, crop resizes) are unchanged.Nudge, then panel edit. The pending-edits registry (
studioPendingEdits.ts) is where every edit starts:trackedStudioEditwraps every Design-panel action, andbeginStudioPendingEditstarts every canvas gesture. Starting an edit there now raises the existing flush event that undo already raises. A debounced edit, such as a nudge burst or a color-grading save, therefore commits before the new edit begins, in the order the user made them.Starting the flushed save first is not enough on an animated element: its save reads and rewrites the GSAP script asynchronously, and a panel edit started right after could still write first. So a Design-panel action or a canvas gesture that starts while a flushed save is still writing waits for that save, then runs. Two such edits keep the order they were made in. Studio's own internal saves (the GSAP script commit, timeline edits) never wait, so a save never waits on itself, including a save made from inside a waiting edit or a flushed one. An edit that runs inside a canvas gesture's own commit, such as a second nudge burst's save, already waited when the gesture started and does not wait again.
The wait. It only happens in the window between a nudge burst's last key and its save landing, and only for an edit made in that window. Measured from W + Enter to the new width on screen, median of 3 runs per row:
Undoing a
gsap.seton an animated element. On a GSAP-animated box, W 300 writesgsap.set("#target", { width: 300 }). Cmd+Z took it out of the file, but the preview and the Design panel stayed at 300. The panel applies W to the live element and writes thesetonly to the file, so the preview's own script never contains it. Undo re-runs the restored script in place, but that reload only reset what the preview's own script had written, so the inlinewidth: 300pxstayed. The same was true for any later edit that removes theset, such as deleting the element's animations.A live patch of a
setvalue goes through one place,applyGlobalSet. It now records the element and the property names it applied, with no parsing. The next in-place reload of that element's composition, from undo or any other edit, resets exactly those properties to their authored value. The re-run script then sets again whatever it still sets, in the same synchronous step, and the record is dropped. The record covers only what a live patch applied. Asetthat the preview's own script ran is read from that script instead: after a page load, such as undoing a W from an earlier session, or after an in-place reload re-ran a script that still had theset. That is at most one parse per reload, and only when the script containsgsap.set(.gsapSoftReload.test.ts"clears what a standalone gsap.set wrote once the new script no longer sets it" is the test that fails without it.Before
Main. Each video ends on the state after one Cmd+Z.
gsap-nudge-main.mp4
before-gsapset-main.mp4
before-nudge-main.mp4
before-resize-main.mp4
After
This branch, the same steps.
gsap-nudge-branch.mp4
after-gsapset-branch.mp4
after-nudge-branch.mp4
after-resize-branch.mp4
Test plan
useDomEditNudge.test.tsx: an arrow-key burst still inside its pause, then a Design-panel edit. The nudge commits first.studioPendingEdits.test.ts: a debounced edit commits before a panel edit and before a canvas gesture begins.studioPendingEdits.test.ts, a panel edit starts only after a flushed save that was still writing has written. Later edits keep their order behind it.useDomEditNudge.test.tsx, a Design-panel edit made during a burst lands after the nudge.gsap.setundo. IngsapUndoRestore.test.ts, undoing a W whosesetthe preview never ran clears the width and keeps the rest of the element's style. It fails at300pxwithout the change.gsapSoftReload.test.tscovers the same reset when the preview's script did run theset. IngsapSoftReload.test.ts, W 300 then H 200 applied live, then a reload whose script sets only the width, clears the height and keeps the rest; a later reload leaves a height the new script set. Removing the record, the reset or the forget each fails it.studioPendingEdits.test.ts, a burst flushed while the first burst is still saving saves after it, and later panel edits land in order. It hangs without the change.useDomGeometryCommits.test.tsx, a resize and the next size edit of the same element record under different undo keys. The test fails on main with the shared keybox-size:index.html||box|#box|and passes with the change.seqdeep-none-px-r0-root-z100, from the parked undo-chain bench work, which a W edit right after a resize fails), on a Linux dev machine with Chrome for Testing:Trade-offs
setreset does not reach. An element the preview cannot find in its file keeps the old behaviour. Agsap.setselector that also matches an element of another inlined composition resets that element too, and only its own script's next run sets it again.Also: Studio's published types name
cnby its packageCarried here from a separate small fix, so it lands with this PR.
@hyperframes/studiotypes importcn's type from'node_modules/cn/dist/types', which does not resolve for a consumer. On some runners the type build refused it (TS2742 oncn.ts), so Studio's build failed intermittently in the Windows render check.cnis annotated withCnFunctionfrom thecnpackage's public entry. This fixes the Windows failure."baseUrl": ".", whose only use was a path alias that resolves without it. This stops such a path shipping silently on Linux.baseUrloncecnis annotated, and Studio's typecheck and build pass.