fix(studio,core): resolve SDK-cutover review findings (#1471)

Co-authored-by: Miguel Ángel <miguel07alm@protonmail.com>
This commit is contained in:
Vance Ingalls
2026-06-17 16:44:45 -07:00
committed by GitHub
co-authored by Miguel Ángel
parent 7ca4490328
commit 377b0368bd
14 changed files with 343 additions and 338 deletions
@@ -31,6 +31,19 @@ function findGsapScriptElements(doc: Document): HTMLScriptElement[] {
return results;
}
/**
* Extract the GSAP timeline script text from a serialized HTML document, for
* feeding into applySoftReload. Returns null when zero or multiple GSAP scripts
* are present (ambiguous — caller should fall back to a full reload), matching
* applySoftReload's own single-script requirement.
*/
export function extractGsapScriptText(html: string): string | null {
const doc = new DOMParser().parseFromString(html, "text/html");
const scripts = findGsapScriptElements(doc);
if (scripts.length !== 1) return null;
return scripts[0].textContent || null;
}
/** Check that the new script repopulated __timelines with at least one entry. */
function verifyTimelinesPopulated(win: IframeWindow): boolean {
const tlKeys = win.__timelines
@@ -73,6 +86,7 @@ export function applySoftReload(iframe: HTMLIFrameElement | null, scriptText: st
// full iframe reload that destroys the very WebGL context we're preserving.
let deferredToAsync = false;
// fallow-ignore-next-line complexity
const doReload = () => {
const timelines = win.__timelines;
const allTargets: Element[] = [];
+14 -2
View File
@@ -104,7 +104,12 @@ describe("sdkCutoverPersist", () => {
({
getElement: vi.fn().mockReturnValue(hasEl ? { inlineStyles: {} } : null),
dispatch: vi.fn(),
serialize: vi.fn().mockReturnValue("<html></html>"),
// Distinct before/after so the no-op guard (after === before → fall back)
// treats this as a real change; "after" matches the write assertions.
serialize: vi
.fn()
.mockReturnValueOnce("<html>before</html>")
.mockReturnValue("<html></html>"),
batch: vi.fn((fn: () => void) => fn()),
}) as unknown as Parameters<typeof sdkCutoverPersist>[4];
@@ -318,7 +323,11 @@ describe("sdkDeletePersist", () => {
({
getElement: vi.fn().mockReturnValue(hasEl ? { id: "hf-abc" } : null),
removeElement: vi.fn(),
serialize: vi.fn().mockReturnValue("<html>after</html>"),
serialize: vi
.fn()
.mockReturnValueOnce("<html>before-snap</html>")
.mockReturnValue("<html>after</html>"),
batch: vi.fn((fn: () => void) => fn()),
}) as unknown as Parameters<typeof sdkDeletePersist>[3];
it("returns false when session is null", async () => {
@@ -390,6 +399,7 @@ describe("sdkTimingPersist", () => {
.fn()
.mockReturnValueOnce("<html>before</html>")
.mockReturnValue("<html>after</html>"),
batch: vi.fn((fn: () => void) => fn()),
}) as unknown as Parameters<typeof sdkTimingPersist>[3];
it("returns false when session is null", async () => {
@@ -466,6 +476,7 @@ describe("sdkGsapTweenPersist", () => {
.fn()
.mockReturnValueOnce("<html>before</html>")
.mockReturnValue("<html>after</html>"),
batch: vi.fn((fn: () => void) => fn()),
}) as unknown as Parameters<typeof sdkGsapTweenPersist>[2];
it("returns false when session is null", async () => {
@@ -573,6 +584,7 @@ describe("sdkGsapKeyframePersist", () => {
.fn()
.mockReturnValueOnce("<html>before</html>")
.mockReturnValue("<html>after</html>"),
batch: vi.fn((fn: () => void) => fn()),
}) as unknown as Parameters<typeof sdkGsapKeyframePersist>[4];
it("returns false when session is null", async () => {
+69 -21
View File
@@ -64,7 +64,7 @@ export function shouldUseSdkCutover(
);
}
interface CutoverDeps {
export interface CutoverDeps {
editHistory: {
recordEdit: (entry: {
label: string;
@@ -76,22 +76,44 @@ interface CutoverDeps {
writeProjectFile: (path: string, content: string) => Promise<void>;
reloadPreview: () => void;
domEditSaveTimestampRef: MutableRefObject<number>;
/**
* Optional post-write refresh. When provided, it REPLACES the default
* reloadPreview() — the GSAP path passes one that soft-reloads (preserving
* the playhead) and invalidates the keyframe/gsap panel cache. Receives the
* serialized document just written.
*/
refresh?: (after: string) => void;
/**
* Path of the composition the SDK session was opened for. The session models
* ONLY this file (serialize() emits the whole active composition), so any edit
* whose targetPath differs (a sub-composition file) must take the server path
* — otherwise we'd write the full active-comp serialization into that file.
*/
compositionPath?: string | null;
}
/** True when targetPath isn't the composition the SDK session models. */
function wrongCompositionFile(deps: CutoverDeps, targetPath: string): boolean {
return deps.compositionPath != null && targetPath !== deps.compositionPath;
}
interface CutoverOptions {
label?: string;
coalesceKey?: string;
/** Skip the preview reload (mirrors the server path's skipRefresh). */
skipRefresh?: boolean;
}
// ponytail: internal; export only if a third caller appears
// ponytail: internal; export only if a third caller appears.
// `after` is serialized once by the caller (which also did the no-op check
// against its pre-dispatch snapshot), so this never re-serializes.
async function persistSdkSerialize(
sdkSession: Composition,
after: string,
targetPath: string,
originalContent: string,
deps: CutoverDeps,
options?: CutoverOptions,
): Promise<void> {
const after = sdkSession.serialize();
deps.domEditSaveTimestampRef.current = Date.now();
await deps.writeProjectFile(targetPath, after);
await deps.editHistory.recordEdit({
@@ -100,7 +122,8 @@ async function persistSdkSerialize(
...(options?.coalesceKey ? { coalesceKey: options.coalesceKey } : {}),
files: { [targetPath]: { before: originalContent, after } },
});
deps.reloadPreview();
if (deps.refresh) deps.refresh(after);
else if (!options?.skipRefresh) deps.reloadPreview();
}
export async function sdkCutoverPersist(
@@ -118,13 +141,17 @@ export async function sdkCutoverPersist(
const hfId = selection.hfId;
if (!hfId) return false;
if (!sdkSession.getElement(hfId)) return false;
if (wrongCompositionFile(deps, targetPath)) return false;
try {
const before = sdkSession.serialize();
sdkSession.batch(() => {
for (const editOp of patchOpsToSdkEditOps(hfId, ops)) {
sdkSession.dispatch(editOp);
}
});
await persistSdkSerialize(sdkSession, targetPath, originalContent, deps, options);
const after = sdkSession.serialize();
if (after === before) return false;
await persistSdkSerialize(after, targetPath, originalContent, deps, options);
trackStudioEvent("sdk_cutover_success", { hfId, opCount: ops.length });
return true;
} catch (err) {
@@ -145,10 +172,13 @@ export async function sdkTimingPersist(
options?: CutoverOptions,
): Promise<boolean> {
if (!sdkSession || !sdkSession.getElement(hfId)) return false;
if (wrongCompositionFile(deps, targetPath)) return false;
try {
const before = sdkSession.serialize();
sdkSession.setTiming(hfId, timingUpdate);
await persistSdkSerialize(sdkSession, targetPath, before, deps, options);
sdkSession.batch(() => sdkSession.setTiming(hfId, timingUpdate));
const after = sdkSession.serialize();
if (after === before) return false;
await persistSdkSerialize(after, targetPath, before, deps, options);
trackStudioEvent("sdk_cutover_success", { hfId, opCount: 1 });
return true;
} catch (err) {
@@ -170,17 +200,26 @@ export async function sdkGsapTweenPersist(
options?: CutoverOptions,
): Promise<boolean> {
if (!sdkSession) return false;
if (wrongCompositionFile(deps, targetPath)) return false;
try {
if (op.kind === "add" && !sdkSession.getElement(op.target)) return false;
const before = sdkSession.serialize();
if (op.kind === "add") {
if (!sdkSession.getElement(op.target)) return false;
sdkSession.addGsapTween(op.target, op.spec);
} else if (op.kind === "set") {
sdkSession.setGsapTween(op.animationId, op.properties);
} else {
sdkSession.removeGsapTween(op.animationId);
}
await persistSdkSerialize(sdkSession, targetPath, before, deps, options);
sdkSession.batch(() => {
if (op.kind === "add") {
sdkSession.addGsapTween(op.target, op.spec);
} else if (op.kind === "set") {
sdkSession.setGsapTween(op.animationId, op.properties);
} else {
sdkSession.removeGsapTween(op.animationId);
}
});
const after = sdkSession.serialize();
// No-op (stale animationId, unsupported shape e.g. from-prop on a plain
// tween): fall back to the server path so it surfaces the proper error
// instead of writing a phantom before==after undo step. Subsumes a
// per-op existence guard for the set/remove branches.
if (after === before) return false;
await persistSdkSerialize(after, targetPath, before, deps, options);
trackStudioEvent("sdk_cutover_success", { opCount: 1 });
return true;
} catch (err) {
@@ -199,10 +238,15 @@ export async function sdkGsapKeyframePersist(
options?: CutoverOptions,
): Promise<boolean> {
if (!sdkSession) return false;
if (wrongCompositionFile(deps, targetPath)) return false;
try {
const before = sdkSession.serialize();
sdkSession.dispatch({ type: "addGsapKeyframe", animationId, position, value });
await persistSdkSerialize(sdkSession, targetPath, before, deps, options);
sdkSession.batch(() =>
sdkSession.dispatch({ type: "addGsapKeyframe", animationId, position, value }),
);
const after = sdkSession.serialize();
if (after === before) return false;
await persistSdkSerialize(after, targetPath, before, deps, options);
trackStudioEvent("sdk_cutover_success", { opCount: 1 });
return true;
} catch (err) {
@@ -219,9 +263,13 @@ export async function sdkDeletePersist(
deps: CutoverDeps,
): Promise<boolean> {
if (!sdkSession || !sdkSession.getElement(hfId)) return false;
if (wrongCompositionFile(deps, targetPath)) return false;
try {
sdkSession.removeElement(hfId);
await persistSdkSerialize(sdkSession, targetPath, originalContent, deps, {
const before = sdkSession.serialize();
sdkSession.batch(() => sdkSession.removeElement(hfId));
const after = sdkSession.serialize();
if (after === before) return false;
await persistSdkSerialize(after, targetPath, originalContent, deps, {
label: "Delete element",
});
trackStudioEvent("sdk_cutover_success", { hfId, opCount: 1 });