Skip to content

refactor(studio): one hook owns file undo and redo - #4189

Merged
miguel-heygen merged 4 commits into
mainfrom
hfoss22/history-actions
Sep 19, 2026
Merged

miguel-heygen merged 4 commits into
mainfrom
hfoss22/history-actions

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

What

File undo and redo now live in one hook, useEditHistoryActions, that useAppHotkeys calls. The caption-mode and beat-history steps stay in useAppHotkeys, ahead of it. Behaviour is unchanged.

Why

Undo and redo were only reachable through the hotkey hook, fused with caption and beat history. A host that mounts Studio's timeline editing had no way to undo through the same path the edits are recorded with. This gives that path a single owner without changing how Studio behaves.

How

  • hooks/useEditHistoryActions.ts: the read, write and serialize wiring, the wait for pending saves, the SDK reload, the preview sync and the toasts, moved as they were.
  • hooks/useAppHotkeys.ts: keeps caption and beat precedence, then calls the hook.
  • hooks/useEditHistoryActions.test.tsx: undo writes through the host writer and resyncs; redo skips the SDK reload for other files; a refused undo explains itself; pending saves are awaited first and the motion file is read through the optional reader.

Before

Delete a clip, then Undo, on a fixture project, on main:

before.mp4

After

The same edit on this branch. This is a refactor, so the behaviour is unchanged by design and the frames match the Before; the clip is deleted and Undo restores it:

after.mp4

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving. The extraction is faithful — I checked it statement by statement against base c610d849, not on the "behaviour unchanged" claim.

Parity ledger — applyHistory (base useAppHotkeys.ts:185-243) → apply (useEditHistoryActions.ts:67-101)

step base head
readHistoryFile motion-path split :168-172 :56-60, same deps
serializeHistoryFiles :179-183 :61-65, same deps
await waitForPendingDomEditSaves() :207 :70
editHistory[direction]({readFile, writeFile, serialize}) :208-212 :71-75
content-mismatch toast + return :213-219 :76-79
onAfterUndoRedo?.()activeCompPath check → forceReloadSdkSession?.()await syncHistoryPreviewAfterApplyshowToast :220-229 :80-87
dep array 10 entries, :231-242 same 10, same order, :89-100

The exits are what an extraction usually loses, and they survived. Both guards that own an early return — the caption block (:194-202) and tryApplyBeatHistory (:205) — stayed in useAppHotkeys ahead of the call (head :168-179), so the only exit that had to move is the content-mismatch return, and it did. await waitForPendingDomEditSaves() moving inside the hook does not change when it runs either: both guards return before fileHistory[direction]() at :181 is reached, which is exactly where it sat relative to them before.

Toast strings are byte-identical — const [noun, verb] = … at :69 reproduces File changed outside Studio. Undo history was not applied. and Undid <label> character for character.

One thing that did not move as-is

The body says the write wiring moved as it was. The writeHistoryFile wrapper (base :173-178) is actually gone, and writeProjectFile is now handed straight to writeFile (:73). That is equivalent — the wrapper only awaited and returned void — and the new test pins it directly (expect(deps.writeProjectFile).toHaveBeenCalledWith("index.html", "before")). A note, not a concern; worth naming because it is the one line a reader comparing the two files will trip over.

The import deletion is the audit

STUDIO_MOTION_PATH and serializeStudioFileMutations are gone from useAppHotkeys.ts's import block outright. With unused imports linted, that is a whole-file proof no other site there still reaches the moved wiring — no half-move left behind.

Coverage

The two existing useAppHotkeys.*.test.tsx files mount with editHistory.undo/redo → {ok: false} and assert nothing about history, so their passing was never evidence for this path — though they do exercise the new call site at mount, and neither needed changing. The new file is the first direct coverage this path has had.

The fourth case is the strongest one: it pins the sequence as ["wait", "undo", "serialized"] and the reader split with a negativereadProjectFile not called with STUDIO_MOTION_PATH. That is what makes the ternary at :57-58 non-vacuous rather than merely executed.

Non-blocking — the stated goal is not reachable at this head

The Why is that a host can import this path instead of writing its own. useEditHistoryActions is not among the 27 exports in packages/studio/src/index.ts, and the package exports map admits only ., ./tailwind-preset, ./theme.css and ./package.json, so a deep subpath import is blocked as well. Consistent with this being the first of the timeline-editing export PRs — raising it only so the series does not finish with the hook still private.

Nit

A result with ok: false and any reason other than content-mismatch falls through both branches silently. That is unchanged from before, and nothing pins it. One more mount({ ok: false, reason: "…" }) case asserting no toast would close the last uncovered exit.

— Rames

@miguel-heygen
miguel-heygen merged commit fd6049f into main Sep 19, 2026
81 of 85 checks passed
@miguel-heygen
miguel-heygen deleted the hfoss22/history-actions branch September 19, 2026 19:33
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.

2 participants