refactor(studio): one hook owns file undo and redo - #4189
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
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 syncHistoryPreviewAfterApply → showToast |
: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 negative — readProjectFile 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
What
File undo and redo now live in one hook,
useEditHistoryActions, thatuseAppHotkeyscalls. The caption-mode and beat-history steps stay inuseAppHotkeys, 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