Skip to content

Commit a7c23ce

Browse files
committed
fix(studio): undo of a nested edit reloads when its composition has a GSAP script
1 parent bc3d51b commit a7c23ce

8 files changed

Lines changed: 96 additions & 62 deletions

‎packages/studio/src/hooks/useEditHistoryActions.paint.test.tsx‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// @vitest-environment happy-dom
22
import { act, createElement } from "react";
33
import { createRoot } from "react-dom/client";
4-
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
4+
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
55
import { tmpdir } from "node:os";
66
import { join } from "node:path";
7-
import { afterEach, expect, it, vi } from "vitest";
7+
import { afterEach, beforeEach, expect, it, vi } from "vitest";
88
import {
99
createStudioApi,
1010
openProjectHistory,
@@ -20,26 +20,26 @@ const BEFORE = page("10px");
2020
const AFTER = page("50px");
2121

2222
const cleanup: Array<() => unknown> = [];
23+
let scratch = "";
24+
beforeEach(() => {
25+
scratch = mkdtempSync(join(tmpdir(), "hf-undo-paint-"));
26+
});
2327
afterEach(async () => {
2428
for (const step of cleanup.splice(0).reverse()) await step();
29+
rmSync(scratch, { recursive: true, force: true });
2530
vi.unstubAllGlobals();
2631
});
2732

28-
function tempDir(prefix: string): string {
29-
const dir = mkdtempSync(join(tmpdir(), prefix));
30-
cleanup.push(() => rmSync(dir, { recursive: true, force: true }));
31-
return dir;
32-
}
33-
3433
/** Studio's undo wired as App.tsx wires it, over the real history engine, with a live preview document. */
3534
async function studio() {
3635
vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true);
37-
const dir = tempDir("hf-undo-paint-");
36+
const dir = join(scratch, "project");
37+
mkdirSync(dir);
3838
const path = join(dir, "index.html");
3939
writeFileSync(path, BEFORE);
4040
const engine = await openProjectHistory({
4141
projectDir: dir,
42-
historyRoot: tempDir("hf-undo-paint-root-"),
42+
historyRoot: join(scratch, "history"),
4343
});
4444
cleanup.push(() => engine.close());
4545
const api = createStudioApi({

‎packages/studio/src/hooks/useEditHistoryActions.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ interface HistoryFileCallbacks {
2020
export interface EditHistoryHandle {
2121
undo: (cb: HistoryFileCallbacks) => Promise<HistoryResult>;
2222
redo: (cb: HistoryFileCallbacks) => Promise<HistoryResult>;
23-
/** The restore a step would make, when this tab already knows it. */
2423
predict?: (direction: "undo" | "redo") => RestoreFiles | null;
2524
state: {
2625
undo: ReadonlyArray<{ createdAt: number }>;
@@ -35,7 +34,6 @@ export interface UseEditHistoryActionsOptions {
3534
writeProjectFile: (path: string, content: string) => Promise<void>;
3635
showToast: (message: string, tone?: "error" | "info") => void;
3736
syncHistoryPreviewAfterApply: (restore: Pick<HistoryResult, "paths" | "files">) => Promise<void>;
38-
/** Shows a restore on the live preview in this task, or returns false having touched nothing (e.g. a save is pending). */
3937
showHistoryRestoreNow?: (files: RestoreFiles) => boolean;
4038
waitForPendingDomEditSaves: () => Promise<void>;
4139
onAfterUndoRedo?: (restore: Pick<HistoryResult, "paths" | "files">) => void;
@@ -140,7 +138,6 @@ function swapRestore(files: RestoreFiles): RestoreFiles {
140138
);
141139
}
142140

143-
/** The server's restore, diffed from what the preview already shows. */
144141
function fromShown(files: RestoreFiles | undefined, shown: RestoreFiles | null) {
145142
if (!files || !shown) return files;
146143
return Object.fromEntries(

‎packages/studio/src/hooks/usePersistentEditHistory.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ const OWN_ENTRIES_KEPT = 100;
5252

5353
type OwnFiles = Record<string, { before: string; after: string }>;
5454

55-
/** What this tab wrote per entry id, and the steps the last refresh offered, until a claim or step overtakes it. */
5655
function createOwnHistory() {
5756
const own = new Map<string, OwnFiles>();
5857
let next: Record<"undo" | "redo", NextStep | null> | null = null;
@@ -77,7 +76,6 @@ function createOwnHistory() {
7776
if (seen === changes) next = { undo: view.back, redo: view.forward };
7877
},
7978
changes: () => changes,
80-
/** An undo entry's files are the ones it undid, swapped. */
8179
stepped: (entry: { id: string; undoes?: string }) => {
8280
const undone = entry.undoes ? own.get(entry.undoes) : undefined;
8381
if (!undone) return;
@@ -87,7 +85,6 @@ function createOwnHistory() {
8785
]);
8886
remember(entry.id, Object.fromEntries(swapped));
8987
},
90-
/** What this tab last wrote to each file of an entry, which a step over it starts from. */
9188
afterOf: (id: string | undefined): Record<string, string> =>
9289
Object.fromEntries(Object.entries((id && own.get(id)) || {}).map(([p, f]) => [p, f.after])),
9390
predict: (direction: "undo" | "redo"): Record<string, ApplyRestoredFile> | null => {
@@ -265,8 +262,6 @@ export function usePersistentEditHistory({ projectId }: UsePersistentEditHistory
265262
ok: true,
266263
label: reply.entry.label,
267264
paths: changed,
268-
// A step taken before the view caught up read no file first; the server only steps over
269-
// an entry whose files are still as it left them, so that is where this one starts.
270265
files: await restoredFiles(
271266
changed,
272267
{ ...own.afterOf(reply.entry.undoes), ...previous },
@@ -303,7 +298,6 @@ export function usePersistentEditHistory({ projectId }: UsePersistentEditHistory
303298
recordEdit,
304299
undo,
305300
redo,
306-
/** The files a step would restore, from what this tab wrote, while nothing has overtaken the last refresh. */
307301
predict: own.predict,
308302
};
309303
}

‎packages/studio/src/hooks/usePreviewPersistence.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@ import {
1212
type StudioPendingEditsDrainResult,
1313
} from "../utils/studioPendingEdits";
1414
import { trackStudioEvent } from "../utils/studioTelemetry";
15-
import { applyUndoRestoreToPreview, type UndoRestoreFile } from "../utils/gsapUndoRestore";
16-
import { findGsapScriptElements } from "../utils/gsapSoftReload";
15+
import {
16+
applyUndoRestoreToPreview,
17+
restoreHasGsapScript,
18+
type UndoRestoreFile,
19+
} from "../utils/gsapUndoRestore";
1720
import { usePlayerStore } from "../player";
1821
import { syncStoredAutomationFromPreview } from "../player/lib/automationStoreSync";
1922

@@ -224,8 +227,9 @@ export function usePreviewPersistence({
224227
const showHistoryRestoreNow = useCallback(
225228
(files: Record<string, UndoRestoreFile>): boolean => {
226229
if (!domEditSaveQueueRef.current?.isIdle() || hasStudioPendingEdits()) return false;
227-
if (Object.values(files).some((f) => hasGsapScript(f.previous) || hasGsapScript(f.restored)))
228-
return false;
230+
const scripted = (f: UndoRestoreFile) =>
231+
restoreHasGsapScript(f.previous) || restoreHasGsapScript(f.restored);
232+
if (Object.values(files).some(scripted)) return false;
229233
let refused = false;
230234
applyUndoRestoreToPreview(
231235
previewIframeRef.current,
@@ -266,7 +270,3 @@ export function usePreviewPersistence({
266270
showHistoryRestoreNow,
267271
};
268272
}
269-
270-
function hasGsapScript(html: string): boolean {
271-
return findGsapScriptElements(new DOMParser().parseFromString(html, "text/html")).length > 0;
272-
}

‎packages/studio/src/utils/domEditSaveQueue.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ interface DomEditSaveQueueOptions {
2020
export interface DomEditSaveQueue {
2121
enqueue: <T>(save: () => Promise<T>) => Promise<T>;
2222
waitForIdle: () => Promise<DomEditSaveDrainResult>;
23-
/** No save is queued or running. */
2423
isIdle: () => boolean;
2524
reset: () => void;
2625
destroy: () => void;

‎packages/studio/src/utils/gsapUndoRestore.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,23 @@ describe("applyUndoRestoreToPreview", () => {
395395
expect(reloadPreview).toHaveBeenCalledTimes(1);
396396
});
397397

398+
it("full-reloads a sub-composition restore whose file has a GSAP script", () => {
399+
const { iframe, doc } = buildLiveIframe(host(`style="clip-path: inset(0px 40px 0px 0px);"`));
400+
const reloadPreview = vi.fn();
401+
const script = `<script>gsap.timeline().to("#target", { x: 10 });</script>`;
402+
const scripted = (style: string) =>
403+
sub(style).replace("</div></template>", `${script}</div></template>`);
404+
const files = {
405+
[SUB]: {
406+
previous: scripted(`style="clip-path: inset(0px 40px 0px 0px)"`),
407+
restored: scripted(""),
408+
},
409+
};
410+
expect(applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview)).toBe("full");
411+
expect(reloadPreview).toHaveBeenCalledTimes(1);
412+
expect(doc.querySelector('[data-hf-id="hf-t"]')?.getAttribute("style")).toContain("clip-path");
413+
});
414+
398415
it("full-reloads when the restore touches a sub-comp, not the active comp", () => {
399416
const { iframe } = buildLiveIframe(`<div id="a">t</div>`);
400417
const reloadPreview = vi.fn();

‎packages/studio/src/utils/gsapUndoRestore.ts‎

Lines changed: 61 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -175,39 +175,70 @@ function hasAmbiguousGsapScriptChange(previous: string, restored: string): boole
175175
);
176176
}
177177

178-
/**
179-
* Every live element a restore changes, paired with its restored markup, or null when the restore is
180-
* beyond an attribute sync. A file is the active document or a sub-composition the live preview
181-
* inlines into each `[data-composition-file]` host; only the active document's GSAP script may change.
182-
*/
178+
type RestoreTarget = { live: Element; restored: Element };
179+
180+
/** Whether a file, template content included, carries a GSAP script. */
181+
export function restoreHasGsapScript(html: string): boolean {
182+
return findGsapScriptElements(parseRestoreSource(html)).length > 0;
183+
}
184+
185+
// The active document is the whole preview; a sub-composition lives in each host that inlines it.
186+
function liveScopes(doc: Document, path: string, isActive: boolean): ParentNode[] {
187+
if (isActive) return [doc];
188+
return Array.from(doc.querySelectorAll(`[data-composition-file="${CSS.escape(path)}"]`));
189+
}
190+
191+
function scopeTargets(
192+
scope: ParentNode,
193+
keys: string[],
194+
restoredByKey: Map<string, Element>,
195+
): RestoreTarget[] | null {
196+
const liveByKey = identityElementMap(scope);
197+
if (!liveByKey) return null;
198+
const targets: RestoreTarget[] = [];
199+
for (const key of keys) {
200+
const live = liveByKey.get(key);
201+
const restored = restoredByKey.get(key);
202+
// The preview rewrites a sub-composition's own root, so its attributes are not the file's.
203+
if (!live || !restored || live.hasAttribute("data-hf-inner-root")) return null;
204+
targets.push({ live, restored });
205+
}
206+
return targets;
207+
}
208+
209+
function fileTargets(
210+
doc: Document,
211+
path: string,
212+
isActive: boolean,
213+
{ previous, restored }: UndoRestoreFile,
214+
): RestoreTarget[] | null {
215+
// Only the active document's GSAP script can be re-run in place.
216+
if (!isActive && (restoreHasGsapScript(previous) || restoreHasGsapScript(restored))) return null;
217+
const scopes = liveScopes(doc, path, isActive);
218+
const diff = diffSoftReloadableRestore(previous, restored);
219+
if (!scopes.length || !diff || hasAmbiguousGsapScriptChange(previous, restored)) return null;
220+
const restoredByKey = identityElementMap(parseRestoreSource(restored));
221+
if (!restoredByKey) return null;
222+
const targets: RestoreTarget[] = [];
223+
for (const scope of scopes) {
224+
const found = scopeTargets(scope, diff.changedElementKeys, restoredByKey);
225+
if (!found) return null;
226+
targets.push(...found);
227+
}
228+
return targets;
229+
}
230+
231+
/** Every live element a restore changes with its restored markup, or null when it needs a reload. */
183232
function planRestoreTargets(
184233
doc: Document,
185234
activeDocPath: string,
186235
files: Record<string, UndoRestoreFile>,
187-
): Array<{ live: Element; restored: Element }> | null {
188-
const targets: Array<{ live: Element; restored: Element }> = [];
189-
for (const [path, { previous, restored }] of Object.entries(files)) {
190-
const isActive = path === activeDocPath;
191-
const scopes: ParentNode[] = isActive
192-
? [doc]
193-
: Array.from(doc.querySelectorAll(`[data-composition-file="${CSS.escape(path)}"]`));
194-
const diff = diffSoftReloadableRestore(previous, restored);
195-
if (!scopes.length || !diff || hasAmbiguousGsapScriptChange(previous, restored)) return null;
196-
if (!isActive && extractGsapScriptText(previous) !== extractGsapScriptText(restored))
197-
return null;
198-
const restoredByKey = identityElementMap(parseRestoreSource(restored));
199-
if (!restoredByKey) return null;
200-
for (const scope of scopes) {
201-
const liveByKey = identityElementMap(scope);
202-
if (!liveByKey) return null;
203-
for (const key of diff.changedElementKeys) {
204-
const live = liveByKey.get(key);
205-
const restoredEl = restoredByKey.get(key);
206-
// The preview rewrites a sub-composition's own root, so its attributes are not the file's.
207-
if (!live || !restoredEl || live.hasAttribute("data-hf-inner-root")) return null;
208-
targets.push({ live, restored: restoredEl });
209-
}
210-
}
236+
): RestoreTarget[] | null {
237+
const targets: RestoreTarget[] = [];
238+
for (const [path, file] of Object.entries(files)) {
239+
const found = fileTargets(doc, path, path === activeDocPath, file);
240+
if (!found) return null;
241+
targets.push(...found);
211242
}
212243
return targets;
213244
}
@@ -233,10 +264,7 @@ function planRestoreTargets(
233264
* lane move, timing shift, style tweak) → NO script execution — the
234265
* blink-free finalization only (seek + __hfForceTimelineRebind + manual
235266
* reapply, exactly the rebindPreviewTiming path), so timing-attribute
236-
* reverts refresh their visibility windows. Re-running an unchanged
237-
* script here used to be the biggest undo blink source: it tore down
238-
* and rebuilt live timelines (and full-reloaded whenever the script
239-
* couldn't be scoped) for restores that never touched it.
267+
* reverts refresh their visibility windows.
240268
*
241269
* Returns "soft" when applied in place, "full" when it escalated to reloadPreview
242270
* (ineligible restore, missing target, or a permanent soft-reload failure).

‎packages/studio/src/utils/studioPendingEdits.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ function focusedField(): HTMLElement | null {
3535
: null;
3636
}
3737

38-
/** An edit is still saving, or a focused field holds one a flush would commit. */
3938
export function hasStudioPendingEdits(): boolean {
4039
return pendingEditPromises.size > 0 || focusedField() !== null;
4140
}

0 commit comments

Comments
 (0)