From 30b651335afe5105c5dbdfe8a99bcc43d284c5b7 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 15:51:27 -0700 Subject: [PATCH 1/4] fix(studio): stop a group drag from jumping one element back Dragging several elements at once and dropping them made one of them snap back to where it started for a frame or two, then jump forward again. Each member of the group is written separately, and every write patched the live GSAP tween in place and then seeked the player. A seek re-renders the WHOLE timeline, not the tween that changed, so the members still queued behind that write got repainted from their un-patched tweens: back to their pre-drag position, where they sat until their own write landed. Only members whose tween actually renders at the playhead showed it, which is why a group of three flashed one element and left the others still. The group commit now defers the seek for every member but the last, so the queued members keep the transform the gesture left on them and the whole group repaints once, from the fully patched timeline. --- .../src/components/editor/manualOffsetDrag.ts | 2 + .../studio/src/hooks/gsapRuntimePatch.test.ts | 43 +++++++++++++++++++ packages/studio/src/hooks/gsapRuntimePatch.ts | 12 +++++- .../studio/src/hooks/gsapScriptCommitTypes.ts | 10 +++++ .../studio/src/hooks/useGsapAwareEditing.ts | 12 +++++- .../src/hooks/useGsapScriptCommits.test.tsx | 10 ++++- .../studio/src/hooks/useGsapScriptCommits.ts | 4 ++ 7 files changed, 89 insertions(+), 4 deletions(-) diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index 731c41813f..bb4b7df792 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -524,6 +524,7 @@ export function restoreManualOffsetDragMembers(members: ManualOffsetDragMember[] } } +/** Teardown after a COMMITTED drag. */ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): void { for (const member of members) { endStudioManualEditGesture(member.element, member.gestureToken); @@ -552,6 +553,7 @@ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): v } } +/** Release the timelines this gesture paused, re-rendering at the playhead. */ export function resumeGsapTimelines(element: HTMLElement): void { const ids = element.getAttribute("data-hf-drag-paused-timelines"); element.removeAttribute("data-hf-drag-paused-timelines"); diff --git a/packages/studio/src/hooks/gsapRuntimePatch.test.ts b/packages/studio/src/hooks/gsapRuntimePatch.test.ts index 1a17f7e063..e34d293a41 100644 --- a/packages/studio/src/hooks/gsapRuntimePatch.test.ts +++ b/packages/studio/src/hooks/gsapRuntimePatch.test.ts @@ -523,3 +523,46 @@ describe("patchRuntimeTweenInPlace — composition isolation", () => { expect(otherTween.invalidate).not.toHaveBeenCalled(); }); }); + +describe("patchRuntimeTweenInPlace — deferSeek", () => { + /** + * A group drag commits one member at a time. Each in-place patch used to seek, + * and a seek re-renders the WHOLE timeline — so every member still queued behind + * the current one got repainted from its un-patched tween, back to where it sat + * before the drag, and stayed there until its own patch landed. That is the jump. + */ + it("does not seek while a group commit is still writing its other members", () => { + const a = { id: "a" }; + const rendered = { a: 0, b: 0 }; + const tweenA = makeTween({ vars: { x: 0 }, targetIds: ["a"], duration: 0 }, a); + const tweenB = makeTween({ vars: { x: 0 }, targetIds: ["b"], duration: 0 }, a); + const { iframe, seek } = fakeIframe(a, [tweenA, tweenB], { + onSeek: () => { + rendered.a = tweenA.vars.x as number; + rendered.b = tweenB.vars.x as number; + }, + }); + + const first = patchRuntimeTweenInPlace( + iframe, + "#a", + { kind: "set", props: { x: 500 } }, + undefined, + true, + ); + + expect(first).toBe(true); + expect(tweenA.vars.x).toBe(500); + // No repaint yet: "b" keeps the transform the gesture left on it instead of + // being rendered from its own tween, which still holds the pre-drag value. + expect(seek).not.toHaveBeenCalled(); + expect(rendered).toEqual({ a: 0, b: 0 }); + + tweenB.vars.x = 600; + const last = patchRuntimeTweenInPlace(iframe, "#a", { kind: "set", props: { x: 500 } }); + + expect(last).toBe(true); + expect(seek).toHaveBeenCalledTimes(1); + expect(rendered).toEqual({ a: 500, b: 600 }); + }); +}); diff --git a/packages/studio/src/hooks/gsapRuntimePatch.ts b/packages/studio/src/hooks/gsapRuntimePatch.ts index de2c6480e4..9d32650387 100644 --- a/packages/studio/src/hooks/gsapRuntimePatch.ts +++ b/packages/studio/src/hooks/gsapRuntimePatch.ts @@ -277,12 +277,16 @@ function applyChange(tween: RuntimeTween, change: RuntimeTweenChange): boolean { /** * Edit one tween in `window.__timelines` in place + re-seek to the current playhead. * Returns `true` on a confident patch, `false` otherwise (caller soft-reloads). + * + * `deferSeek` skips the re-render, for a caller patching several tweens in a row + * that will render once after the last one. */ export function patchRuntimeTweenInPlace( iframe: HTMLIFrameElement | null, selector: string, change: RuntimeTweenChange, compositionId?: string, + deferSeek = false, ): boolean { if (!iframe) return false; // A base `gsap.set` has no timeline tween to resolve — apply the value straight @@ -312,7 +316,13 @@ export function patchRuntimeTweenInPlace( if (change.kind !== "keyframe-rebuild") { tween.invalidate?.(); } - seekToCurrent(iframe, timeline); + // A seek re-renders the WHOLE timeline, not just the tween we patched. Under a + // multi-element commit that is a visible jump: the members still queued behind + // this one get repainted from their un-patched tweens, back to where they were + // before the gesture, and stay there until their own patch lands. Deferring + // leaves them showing the gesture's own transform, and the caller's last patch + // seeks once for the whole group. + if (!deferSeek) seekToCurrent(iframe, timeline); return true; } catch { return false; diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index 929e79b8ec..c36656e874 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -22,6 +22,16 @@ export interface CommitMutationOptions { coalesceMs?: number; softReload?: boolean; skipReload?: boolean; + /** + * Write the source but leave the preview alone; the caller renders once when it + * is done. For a multi-write action like a group drag, rendering after each + * write shows a source where the members not yet written still hold their old + * values, so they snap back until their own write lands. This also defers the + * in-place runtime patch's seek, which re-renders the whole timeline and repaints + * the queued members the same way. Unlike `skipReload` this changes nothing about + * error handling — a failed write still throws. + */ + deferPreviewSync?: boolean; beforeReload?: () => void; /** * Serialize this commit against others sharing the same key. Used to chain diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 0e84c15826..5e85d00765 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -155,11 +155,20 @@ export function useGsapAwareEditing({ // it survives the N sequential server round-trips) onto each commit — // otherwise each member records its own entry and it takes N presses to undo. const coalesceKey = `group-drag:${++groupDragCommitCounter}`; + // Members are written one at a time, and a write that re-renders the preview + // re-runs the whole script — which still holds the OLD position of every + // member not yet written. Those members snap back to where they started and + // stay there until their own write lands, which is the single element seen + // jumping mid-commit while the rest of the group sat still. The drafted + // positions are already on screen, so holding the render until the last + // member has been written costs nothing and never shows a half-moved group. + let renderOnCommit = false; const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => gsapCommitMutation(selection, mutation, { ...options, coalesceKey, coalesceMs: Number.POSITIVE_INFINITY, + deferPreviewSync: !renderOnCommit, }); const preflightAnimations = new Map(); // Editability is user-atomic: prove every member can be written before @@ -185,7 +194,8 @@ export function useGsapAwareEditing({ throw error; } } - for (const { selection, next } of updates) { + for (const [index, { selection, next }] of updates.entries()) { + renderOnCommit = index === updates.length - 1; try { const outcome = await tryGsapDragIntercept( selection, diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 6236d032ab..b5707e3a7d 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -76,7 +76,10 @@ describe("applyPreviewSync", () => { expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { kind: "set", props: { x: 10 }, - }); + }, + undefined, + false, + ); expect(applySoftReload).not.toHaveBeenCalled(); expect(reloadPreview).not.toHaveBeenCalled(); }); @@ -341,7 +344,10 @@ describe("runCommit — instantPatch wiring", () => { expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { kind: "set", props: { x: 485, y: 311 }, - }); + }, + undefined, + false, + ); expect(deps.reloadPreview).not.toHaveBeenCalled(); }); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index f1a21a3afe..74a524fd4d 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -254,6 +254,8 @@ export function applyPreviewSync( iframe, options.instantPatch.selector, options.instantPatch.change, + undefined, + options.deferPreviewSync === true, ); // Patched in place — element is already correct on screen; no reload needed. if (patched) return; @@ -262,6 +264,8 @@ export function applyPreviewSync( trackStudioEvent("gsap_instant_patch_fallback", { selector: options.instantPatch.selector }); // Fall through to the soft/full reload path below. } + // Written, but the caller has more writes to make and will render after the last. + if (options.deferPreviewSync) return; if (options.softReload && result.scriptText) { // A soft-reloadable edit escalates to a full iframe remount ONLY on the // PERMANENT "cannot-soft-reload" result (the preview is genuinely stale/ From a831836dbbb3200943a0986bf0421e25cbe1120b Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 16:03:14 -0700 Subject: [PATCH 2/4] perf(studio): commit a group drag in one request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dragging N elements cost N writes and 9 reads for a three-element group: each member fetched the composition's parse to preflight, fetched it again to resolve its tween, then wrote the file on its own round trip. Every one of those writes re-read, re-parsed and re-serialized the whole composition. Three changes, same behaviour: - The parse endpoint shares an in-flight request per file, so callers asking for the same composition at the same moment get one request. Only overlapping calls share — the entry is dropped as soon as it settles, so a read after a write still gets a fresh parse. - The group preflight runs its members together instead of one at a time. A preflight writes nothing, so there is nothing to order. - Members' mutations are queued and sent as one batch write. Anything that re-reads the file flushes the queue first, so a member resolving a shared or stale tween never reads a composition missing writes it is about to build on. The batch carries each member's runtime patch, and only the last one re-renders. A three-element group drag now issues 2 reads and 1 write, down from 9 and 3. --- .../studio/src/hooks/gsapScriptCommitTypes.ts | 8 ++ .../src/hooks/keyframeCacheAstLoad.test.ts | 78 +++++++++++++++ .../studio/src/hooks/keyframeCacheAstLoad.ts | 27 ++++- .../studio/src/hooks/useGsapAwareEditing.ts | 99 ++++++++++++++----- .../src/hooks/useGsapScriptCommits.test.tsx | 79 +++++++++++++-- .../studio/src/hooks/useGsapScriptCommits.ts | 32 +++--- 6 files changed, 276 insertions(+), 47 deletions(-) create mode 100644 packages/studio/src/hooks/keyframeCacheAstLoad.test.ts diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index c36656e874..ed9f4a6ae4 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -49,6 +49,14 @@ export interface CommitMutationOptions { * existing soft/full reload path. Structural edits omit this and reload as before. */ instantPatch?: { selector: string; change: RuntimeTweenChange }; + /** + * The same fast path for a batched commit: one patch per element the batch + * wrote, applied in order. All of them must land for the reload to be skipped + * — one that can't be applied leaves the preview half-patched, so the whole + * batch falls back to the reload. Only the last patch re-renders (see + * `deferSeek`), so a ten-element batch repaints once. + */ + instantPatches?: Array<{ selector: string; change: RuntimeTweenChange }>; } export interface CommitMutationCall { diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts new file mode 100644 index 0000000000..d9a1081df1 --- /dev/null +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { fetchParsedAnimations } from "./keyframeCacheAstLoad"; + +/** + * Parsing a composition is a whole-file read + parse on the server, and a + * multi-element action asks for the same file once per element. Callers that + * overlap in time share one request; a caller that comes after the last one + * settled does not, so a parse issued after a write is never served a + * pre-write answer. + */ +describe("fetchParsedAnimations — in-flight sharing", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function stubFetch(): { calls: () => number; settle: () => void } { + let calls = 0; + const pending: Array<() => void> = []; + vi.stubGlobal("fetch", () => { + calls++; + return new Promise((resolve) => { + pending.push(() => + resolve({ + ok: true, + json: () => Promise.resolve({ animations: [{ id: "a", targetSelector: "#a" }] }), + } as Response), + ); + }); + }); + return { + calls: () => calls, + settle: () => { + for (const release of pending.splice(0, pending.length)) release(); + }, + }; + } + + it("serves overlapping reads of one file from a single request", async () => { + const fetchStub = stubFetch(); + + const pending = [ + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "index.html"), + ]; + fetchStub.settle(); + const results = await Promise.all(pending); + + expect(fetchStub.calls()).toBe(1); + expect(results.map((parsed) => parsed?.animations.length)).toEqual([1, 1, 1]); + }); + + it("does not share across files", async () => { + const fetchStub = stubFetch(); + + const pending = [ + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "other.html"), + ]; + fetchStub.settle(); + await Promise.all(pending); + + expect(fetchStub.calls()).toBe(2); + }); + + it("re-requests once the previous read has settled", async () => { + const fetchStub = stubFetch(); + + const first = fetchParsedAnimations("p", "index.html"); + fetchStub.settle(); + await first; + const second = fetchParsedAnimations("p", "index.html"); + fetchStub.settle(); + await second; + + expect(fetchStub.calls()).toBe(2); + }); +}); diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index 71e38bbb61..e4abc42525 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -42,7 +42,32 @@ function hasAnimations(value: unknown): value is ParsedGsapAnimations { ); } -export async function fetchParsedAnimations( +/** + * Requests for the same file that overlap in time, keyed `projectId|sourceFile`. + * + * Every parse re-reads and re-parses the whole composition server-side, and a + * multi-element action asks for the same file once per element. Sharing the + * in-flight promise makes that one request. Only OVERLAPPING calls share: the + * entry is dropped the moment it settles, so a call made after a write still + * gets a fresh parse. + */ +const inFlightParses = new Map>(); + +export function fetchParsedAnimations( + projectId: string, + sourceFile: string, +): Promise { + const key = `${projectId}|${sourceFile}`; + const inFlight = inFlightParses.get(key); + if (inFlight) return inFlight; + const request = requestParsedAnimations(projectId, sourceFile).finally(() => { + inFlightParses.delete(key); + }); + inFlightParses.set(key, request); + return request; +} + +async function requestParsedAnimations( projectId: string, sourceFile: string, ): Promise { diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 5e85d00765..d23ae3b020 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -24,7 +24,11 @@ import { useGsapSaveFailureTelemetry, useSafeGsapCommitMutation, } from "./useSafeGsapCommitMutation"; -import type { CommitMutation } from "./gsapScriptCommitTypes"; +import type { + CommitMutation, + CommitMutationCall, + CommitMutationOptions, +} from "./gsapScriptCommitTypes"; import { setElementGsapPosition } from "../utils/elementGsap"; import { logResize, logResizeSettle } from "../utils/resizeDebug"; import type { DomEditGroupPathOffsetCommit } from "../components/editor/DomEditOverlay"; @@ -163,37 +167,63 @@ export function useGsapAwareEditing({ // positions are already on screen, so holding the render until the last // member has been written costs nothing and never shows a half-moved group. let renderOnCommit = false; - const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => - gsapCommitMutation(selection, mutation, { - ...options, - coalesceKey, - coalesceMs: Number.POSITIVE_INFINITY, - deferPreviewSync: !renderOnCommit, + const withGroupOptions = (options: CommitMutationOptions): CommitMutationOptions => ({ + ...options, + coalesceKey, + coalesceMs: Number.POSITIVE_INFINITY, + deferPreviewSync: !renderOnCommit, + }); + // Every member writes the same file. Queue their mutations and send them as + // ONE request instead of one round trip per member: the server reads, parses + // and writes the composition once, and the preview patches once. + const queued: CommitMutationCall[] = []; + const flushQueued = async () => { + if (queued.length === 0) return; + const calls = queued.splice(0, queued.length); + if (!gsapCommitMutation.batch) { + for (const call of calls) { + await gsapCommitMutation(call.selection, call.mutation, call.options); + } + return; + } + await gsapCommitMutation.batch(calls, { + ...(calls.at(-1)?.options ?? { label: "Move animated layer (group)" }), + label: "Move animated layer (group)", }); + }; + const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => { + queued.push({ selection, mutation, options: withGroupOptions(options) }); + return Promise.resolve(); + }; const preflightAnimations = new Map(); // Editability is user-atomic: prove every member can be written before // the first source mutation. Network failures after this point retain the // existing multi-request semantics, but a blocked member can never leave // earlier siblings partially moved. - for (const { selection } of updates) { - try { - const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); - preflightAnimations.set(selection, animations); - const outcome = await tryGsapDragIntercept( - selection, - { x: 0, y: 0 }, - animations, - previewIframeRef.current, - coalescedCommit, - undefined, - { preflightOnly: true }, - ); - assertGsapEditPersisted(outcome); - } catch (error) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - throw error; - } - } + // Every member reads the same file, and a preflight writes nothing — so run + // them together. The parse layer shares one in-flight request per file, which + // turns N sequential round trips into one. + await Promise.all( + updates.map(async ({ selection }) => { + try { + const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); + preflightAnimations.set(selection, animations); + const outcome = await tryGsapDragIntercept( + selection, + { x: 0, y: 0 }, + animations, + previewIframeRef.current, + coalescedCommit, + undefined, + { preflightOnly: true }, + ); + assertGsapEditPersisted(outcome); + } catch (error) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + throw error; + } + }), + ); for (const [index, { selection, next }] of updates.entries()) { renderOnCommit = index === updates.length - 1; try { @@ -203,7 +233,13 @@ export function useGsapAwareEditing({ preflightAnimations.get(selection) ?? [], previewIframeRef.current, coalescedCommit, - makeFetchFallback(selection), + // The intercept re-reads the file to resolve a stale or shared tween. + // Anything already queued has to be on disk before that read, or it + // resolves against a file missing writes it is about to build on. + async () => { + await flushQueued(); + return makeFetchFallback(selection)(); + }, { preflightPassed: true }, ); assertGsapEditPersisted(outcome); @@ -212,6 +248,15 @@ export function useGsapAwareEditing({ throw error; } } + try { + await flushQueued(); + } catch (error) { + const selection = updates.at(-1)?.selection; + if (selection) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + } + throw error; + } }, [gsapCommitMutation, previewIframeRef, makeFetchFallback, trackGsapInteractionFailure], ); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index b5707e3a7d..76ca2129da 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -73,10 +73,13 @@ describe("applyPreviewSync", () => { syncDragPreview(result(), reloadPreview); - expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { - kind: "set", - props: { x: 10 }, - }, + expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith( + FAKE_IFRAME, + "#a", + { + kind: "set", + props: { x: 10 }, + }, undefined, false, ); @@ -84,6 +87,63 @@ describe("applyPreviewSync", () => { expect(reloadPreview).not.toHaveBeenCalled(); }); + it("instantPatches: patches every element the batch wrote, rendering once at the end", () => { + patchRuntimeTweenInPlace.mockReturnValue(true); + const reloadPreview = vi.fn(); + + applyPreviewSync( + FAKE_IFRAME, + result(), + { + label: "Move animated layer (group)", + softReload: true, + instantPatches: [ + { selector: "#a", change: { kind: "set" as const, props: { x: 1 } } }, + { selector: "#b", change: { kind: "set" as const, props: { x: 2 } } }, + { selector: "#c", change: { kind: "set" as const, props: { x: 3 } } }, + ], + }, + reloadPreview, + ); + + // Only the last patch re-renders — the earlier two defer their seek, so the + // group repaints once instead of once per member. + expect(patchRuntimeTweenInPlace.mock.calls.map((call) => [call[1], call[4]])).toEqual([ + ["#a", true], + ["#b", true], + ["#c", false], + ]); + expect(applySoftReload).not.toHaveBeenCalled(); + expect(reloadPreview).not.toHaveBeenCalled(); + }); + + it("instantPatches: one patch that misses falls the whole batch back to the reload", () => { + patchRuntimeTweenInPlace.mockImplementation((_iframe, selector) => selector !== "#b"); + applySoftReload.mockReturnValue("applied"); + const reloadPreview = vi.fn(); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Move animated layer (group)", + softReload: true, + instantPatches: [ + { selector: "#a", change: { kind: "set" as const, props: { x: 1 } } }, + { selector: "#b", change: { kind: "set" as const, props: { x: 2 } } }, + ], + }, + reloadPreview, + ); + + // A half-patched preview is worse than a reloaded one: "#a" landed, "#b" did + // not, so the reload repaints both from the written source. + expect(applySoftReload).toHaveBeenCalled(); + expect(trackStudioEvent).toHaveBeenCalledWith("gsap_instant_patch_fallback", { + selector: "#b", + }); + }); + it("instantPatch + patch fails: falls back to the soft reload, passing onAsyncFailure", () => { patchRuntimeTweenInPlace.mockReturnValue(false); applySoftReload.mockReturnValue("applied"); @@ -341,10 +401,13 @@ describe("runCommit — instantPatch wiring", () => { // The file already matched (changed:false) but the runtime patch deferred // from the paired first commit must still land. - expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { - kind: "set", - props: { x: 485, y: 311 }, - }, + expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith( + FAKE_IFRAME, + "#a", + { + kind: "set", + props: { x: 485, y: 311 }, + }, undefined, false, ); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index 74a524fd4d..fb042945d6 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -249,19 +249,24 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - if (options.instantPatch) { - const patched = patchRuntimeTweenInPlace( - iframe, - options.instantPatch.selector, - options.instantPatch.change, - undefined, - options.deferPreviewSync === true, + const patches = options.instantPatches ?? (options.instantPatch ? [options.instantPatch] : []); + if (patches.length > 0) { + const deferSeek = options.deferPreviewSync === true; + const missed = patches.find( + (patch, index) => + !patchRuntimeTweenInPlace( + iframe, + patch.selector, + patch.change, + undefined, + deferSeek || index < patches.length - 1, + ), ); - // Patched in place — element is already correct on screen; no reload needed. - if (patched) return; + // Patched in place — elements are already correct on screen; no reload needed. + if (!missed) return; // The instant path couldn't patch in place — record the fallback so we can // track how often the fast path misses before the soft/full reload below. - trackStudioEvent("gsap_instant_patch_fallback", { selector: options.instantPatch.selector }); + trackStudioEvent("gsap_instant_patch_fallback", { selector: missed.selector }); // Fall through to the soft/full reload path below. } // Written, but the caller has more writes to make and will render after the last. @@ -360,7 +365,12 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra ); if (!result) return; options.onResult?.(result); - await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, options); + // Each call brings its own fast-path patch; the batch wrote them all, so the + // preview sync applies them all rather than just the last call's. + const instantPatches = calls + .map(({ options: callOptions }) => callOptions.instantPatch) + .filter((patch) => patch !== undefined); + await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, instantPatches.length > 0 ? { ...options, instantPatches } : options); }, [showToast, finalizeSuccessfulMutation]); // Every GSAP-script commit is a read-modify-write of one file. Overlapping From efb86d4ce1c35e53f19680b6aed460e4612ff338 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 13:27:29 -0400 Subject: [PATCH 3/4] fix(studio): harden batched drag commits --- .../src/components/editor/manualOffsetDrag.ts | 3 +- .../src/hooks/gestureTransaction.test.ts | 19 ++---- .../studio/src/hooks/gestureTransaction.ts | 27 ++------ .../src/hooks/keyframeCacheAstLoad.test.ts | 30 +++++++++ .../studio/src/hooks/keyframeCacheAstLoad.ts | 5 +- .../hooks/useGsapAnimationFetchFallback.ts | 7 +- .../src/hooks/useGsapAwareEditing.test.tsx | 36 +++++++++++ .../studio/src/hooks/useGsapAwareEditing.ts | 64 ++++++++++++------- .../useGsapInteractionFailureTelemetry.ts | 10 +-- .../src/hooks/useGsapScriptCommits.test.tsx | 25 ++++++++ .../studio/src/hooks/useGsapScriptCommits.ts | 8 ++- packages/studio/src/utils/studioDebug.ts | 2 +- 12 files changed, 169 insertions(+), 67 deletions(-) diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index bb4b7df792..75609a73c3 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -517,6 +517,7 @@ function restoreManualOffsetDragMember(member: ManualOffsetDragMember): void { endStudioManualEditGesture(member.element, member.gestureToken); } +/** Roll back a FAILED drag to the exact gesture-start state. */ export function restoreManualOffsetDragMembers(members: ManualOffsetDragMember[]): void { for (const member of members) { restoreManualOffsetDragMember(member); @@ -553,7 +554,7 @@ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): v } } -/** Release the timelines this gesture paused, re-rendering at the playhead. */ +/** Shared timeline teardown for either the committed or restored path. */ export function resumeGsapTimelines(element: HTMLElement): void { const ids = element.getAttribute("data-hf-drag-paused-timelines"); element.removeAttribute("data-hf-drag-paused-timelines"); diff --git a/packages/studio/src/hooks/gestureTransaction.test.ts b/packages/studio/src/hooks/gestureTransaction.test.ts index 3cc8e45530..8ff3ff28f8 100644 --- a/packages/studio/src/hooks/gestureTransaction.test.ts +++ b/packages/studio/src/hooks/gestureTransaction.test.ts @@ -36,6 +36,7 @@ function runTwoMutationTransaction( describe("runGestureTransaction", () => { beforeEach(() => { trackStudioEventMock.mockReset(); + localStorage.clear(); }); it("settles synchronously before persist reaches its first await", async () => { @@ -249,7 +250,7 @@ describe("runGestureTransaction", () => { .spyOn(element, "getBoundingClientRect") .mockReturnValueOnce(rect(10.04, 20.05, 100.05, 80.05)) .mockReturnValueOnce(rect(11.19, 17.89, 100.29, 78.99)); - const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); const now = vi.spyOn(performance, "now").mockReturnValueOnce(50).mockReturnValueOnce(58.44); await runGestureTransaction({ @@ -261,13 +262,7 @@ describe("runGestureTransaction", () => { }); expect(getRect).toHaveBeenCalledTimes(2); - expect(error).toHaveBeenCalledWith( - "[hf-commit] persist changed pixels", - expect.objectContaining({ - label: "Resize layer", - delta: expect.objectContaining({ x: expect.any(Number) }), - }), - ); + expect(log).not.toHaveBeenCalled(); expect(trackStudioEventMock).toHaveBeenCalledWith("commit_invariant_violation", { label: "Resize layer", delta_x: 1.2, @@ -283,13 +278,13 @@ describe("runGestureTransaction", () => { expect.objectContaining({ pixel_asserted: true }), ); now.mockRestore(); - error.mockRestore(); + log.mockRestore(); }); it("skips the pixel assertion for live position tweens", async () => { const element = document.createElement("div"); const getRect = vi.spyOn(element, "getBoundingClientRect"); - const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); await runGestureTransaction({ element, @@ -301,11 +296,11 @@ describe("runGestureTransaction", () => { }); expect(getRect).not.toHaveBeenCalled(); - expect(error).not.toHaveBeenCalledWith("[hf-commit] persist changed pixels", expect.anything()); + expect(log).not.toHaveBeenCalled(); expect(trackStudioEventMock).not.toHaveBeenCalledWith( "commit_invariant_violation", expect.anything(), ); - error.mockRestore(); + log.mockRestore(); }); }); diff --git a/packages/studio/src/hooks/gestureTransaction.ts b/packages/studio/src/hooks/gestureTransaction.ts index 4b398d6636..da7d61f487 100644 --- a/packages/studio/src/hooks/gestureTransaction.ts +++ b/packages/studio/src/hooks/gestureTransaction.ts @@ -4,6 +4,7 @@ import type { CommitMutationOptions, } from "./gsapScriptCommitTypes"; import { trackStudioEvent } from "../utils/studioTelemetry"; +import { makeStudioDebugLogger } from "../utils/studioDebug"; type PixelRect = Pick; @@ -108,14 +109,7 @@ async function dispatchBufferedCommits(calls: BufferedCommit[]): Promise return reloadsRequested(calls); } -/** - * Dev-only [hf-commit] lifecycle trace. The production observability lives in - * the trackStudioEvent commit_* events (always on); these console lines are a - * developer aid and stay out of end users' consoles. - */ -function traceCommit(stage: string, data: Record): void { - if (import.meta.env.DEV) console.info(`[hf-commit] ${stage}`, data); -} +const logCommit = makeStudioDebugLogger("commit"); /** * Owns the visual + persistence + history lifecycle for one gesture release. @@ -127,9 +121,9 @@ export function runGestureTransaction(tx: GestureTransaction): Promise { let mutationCount = 0; let reloadCount = 0; const bufferedCommits: BufferedCommit[] = []; - traceCommit("start", { label: tx.label, coalesceKey }); + logCommit("start", { label: tx.label, coalesceKey }); tx.settle(); - traceCommit("settled", { label: tx.label, coalesceKey }); + logCommit("settled", { label: tx.label, coalesceKey }); const before = !tx.skipPixelAssert ? readPixelRect(tx.element) : null; const commit: TxCommit = (commitMutation) => { @@ -152,19 +146,12 @@ export function runGestureTransaction(tx: GestureTransaction): Promise { .then(async () => { reloadCount = await dispatchBufferedCommits(bufferedCommits); const durationMs = Math.round(performance.now() - startedAt); - traceCommit("persisted", { label: tx.label, coalesceKey }); + logCommit("persisted", { label: tx.label, coalesceKey }); if (before) { const after = readPixelRect(tx.element); const delta = pixelDelta(before, after); if (exceedsPixelTolerance(delta)) { - if (import.meta.env.DEV) { - console.error("[hf-commit] persist changed pixels", { - label: tx.label, - before, - after, - delta, - }); - } + logCommit("persist-changed-pixels", { label: tx.label, before, after, delta }); trackStudioEvent("commit_invariant_violation", { label: tx.label, delta_x: roundToOneDecimal(delta.x), @@ -193,7 +180,7 @@ export function runGestureTransaction(tx: GestureTransaction): Promise { error_name: error instanceof Error ? error.name : "unknown", restore_ran: true, }); - traceCommit("restore", { label: tx.label, coalesceKey }); + logCommit("restore", { label: tx.label, coalesceKey }); throw error; }); } diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts index d9a1081df1..05d5c95d4c 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts @@ -75,4 +75,34 @@ describe("fetchParsedAnimations — in-flight sharing", () => { expect(fetchStub.calls()).toBe(2); }); + + it("supersedes an in-flight pre-write parse with a fresh post-write read", async () => { + const releases: Array<(response: Response) => void> = []; + const fetch = vi.fn( + () => + new Promise((resolve) => { + releases.push(resolve); + }), + ); + vi.stubGlobal("fetch", fetch); + const response = (id: string) => + ({ + ok: true, + json: () => Promise.resolve({ animations: [{ id, targetSelector: `#${id}` }] }), + }) as Response; + + const stale = fetchParsedAnimations("p", "index.html"); + const fresh = fetchParsedAnimations("p", "index.html", { fresh: true }); + expect(fetch).toHaveBeenCalledTimes(2); + + releases[0]?.(response("stale")); + await stale; + const overlappingFreshRead = fetchParsedAnimations("p", "index.html"); + expect(fetch).toHaveBeenCalledTimes(2); + + releases[1]?.(response("fresh")); + const [freshResult, sharedResult] = await Promise.all([fresh, overlappingFreshRead]); + expect(freshResult?.animations[0]?.id).toBe("fresh"); + expect(sharedResult?.animations[0]?.id).toBe("fresh"); + }); }); diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index e4abc42525..390208e328 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -56,12 +56,15 @@ const inFlightParses = new Map>(); export function fetchParsedAnimations( projectId: string, sourceFile: string, + options: { fresh?: boolean } = {}, ): Promise { const key = `${projectId}|${sourceFile}`; + if (options.fresh) inFlightParses.delete(key); const inFlight = inFlightParses.get(key); if (inFlight) return inFlight; const request = requestParsedAnimations(projectId, sourceFile).finally(() => { - inFlightParses.delete(key); + // A superseded pre-write request must not evict the fresh post-write one. + if (inFlightParses.get(key) === request) inFlightParses.delete(key); }); inFlightParses.set(key, request); return request; diff --git a/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts b/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts index 8dd9ba9757..f7ad8529f1 100644 --- a/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts +++ b/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts @@ -33,6 +33,8 @@ export type ElementAnimationsOutcome = export interface GsapAnimationFetchOptions { /** Refuse the edit when the parse endpoint is unavailable instead of treating it as no motion. */ failOnFetchError?: boolean; + /** Ignore an overlapping pre-write parse and read the source after a durable write. */ + fresh?: boolean; } /** @@ -56,11 +58,13 @@ async function fetchElementAnimationsWithRetry( gsapSourceFile: string, target: { id: string | null; selector: string | null }, failOnFetchError: boolean, + fresh: boolean, ): Promise { let coldAttempts = 0; let errorAttempts = 0; for (;;) { - const parsed = await fetchParsedAnimations(projectId, gsapSourceFile); + const parsed = await fetchParsedAnimations(projectId, gsapSourceFile, { fresh }); + fresh = false; const outcome = selectElementAnimationsOrRetry(parsed, target); if (outcome.kind === "resolved") return outcome.animations; if (outcome.kind === "fetch-error") { @@ -89,6 +93,7 @@ export function useGsapAnimationFetchFallback(projectId: string | null, gsapSour gsapSourceFile, target, options?.failOnFetchError === true, + options?.fresh === true, ); }, [projectId, gsapSourceFile], diff --git a/packages/studio/src/hooks/useGsapAwareEditing.test.tsx b/packages/studio/src/hooks/useGsapAwareEditing.test.tsx index 89ab3f6fdc..9d16157b2d 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.test.tsx +++ b/packages/studio/src/hooks/useGsapAwareEditing.test.tsx @@ -301,6 +301,42 @@ describe("useGsapAwareEditing anchored resize", () => { act(() => root.unmount()); }); + it("reports only the first group preflight failure in input order", async () => { + const failures = [new Error("first blocked"), new Error("second blocked")]; + const trackGsapInteractionFailure = vi.fn(); + const priorDragImplementation = mocks.drag.getMockImplementation(); + mocks.drag.mockImplementation(async (selection) => { + throw selection.id === "a" ? failures[0] : failures[1]; + }); + const { groupCommit, root } = mountGroupHandler({ + gsapCommitMutation: vi.fn().mockResolvedValue(undefined), + makeFetchFallback: () => vi.fn().mockResolvedValue([]), + trackGsapInteractionFailure, + }); + const updates = [ + { + selection: { element: document.createElement("div"), id: "a", selector: "#a" }, + next: { x: 10, y: 10 }, + }, + { + selection: { element: document.createElement("div"), id: "b", selector: "#b" }, + next: { x: 20, y: 20 }, + }, + ] as unknown as DomEditGroupPathOffsetCommit[]; + + await expect(groupCommit(updates)).rejects.toBe(failures[0]); + expect(trackGsapInteractionFailure).toHaveBeenCalledOnce(); + expect(trackGsapInteractionFailure).toHaveBeenCalledWith( + failures[0], + updates[0]?.selection, + "drag", + "Move animated layer (group)", + ); + mocks.drag.mockReset(); + if (priorDragImplementation) mocks.drag.mockImplementation(priorDragImplementation); + act(() => root.unmount()); + }); + it("restores once when resize persistence fails", async () => { const error = new Error("resize failed"); const restore = vi.fn(); diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index d23ae3b020..094afa0278 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -41,6 +41,18 @@ import type { GsapAnimationFetchOptions } from "./useGsapAnimationFetchFallback" // into one another's undo entry (module-local counter, not Date.now()). let groupDragCommitCounter = 0; +function firstPreflightFailure( + results: PromiseSettledResult[], + updates: DomEditGroupPathOffsetCommit[], +): { error: unknown; selection: DomEditSelection } | null { + for (const [index, result] of results.entries()) { + if (result.status !== "rejected") continue; + const selection = updates[index]?.selection; + if (selection) return { error: result.reason, selection }; + } + return null; +} + export interface UseGsapAwareEditingParams { domEditSelection: DomEditSelection | null; selectedGsapAnimations: GsapAnimation[]; @@ -54,7 +66,7 @@ export interface UseGsapAwareEditingParams { ) => () => Promise; trackGsapInteractionFailure: ( error: unknown, - selection: DomEditSelection, + selection: DomEditSelection | null, mutationType: string, label: string, ) => void; @@ -203,27 +215,32 @@ export function useGsapAwareEditing({ // Every member reads the same file, and a preflight writes nothing — so run // them together. The parse layer shares one in-flight request per file, which // turns N sequential round trips into one. - await Promise.all( + const preflightResults = await Promise.allSettled( updates.map(async ({ selection }) => { - try { - const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); - preflightAnimations.set(selection, animations); - const outcome = await tryGsapDragIntercept( - selection, - { x: 0, y: 0 }, - animations, - previewIframeRef.current, - coalescedCommit, - undefined, - { preflightOnly: true }, - ); - assertGsapEditPersisted(outcome); - } catch (error) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - throw error; - } + const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); + preflightAnimations.set(selection, animations); + const outcome = await tryGsapDragIntercept( + selection, + { x: 0, y: 0 }, + animations, + previewIframeRef.current, + coalescedCommit, + undefined, + { preflightOnly: true }, + ); + assertGsapEditPersisted(outcome); }), ); + const preflightFailure = firstPreflightFailure(preflightResults, updates); + if (preflightFailure) { + trackGsapInteractionFailure( + preflightFailure.error, + preflightFailure.selection, + "drag", + "Move animated layer (group)", + ); + throw preflightFailure.error; + } for (const [index, { selection, next }] of updates.entries()) { renderOnCommit = index === updates.length - 1; try { @@ -238,7 +255,7 @@ export function useGsapAwareEditing({ // resolves against a file missing writes it is about to build on. async () => { await flushQueued(); - return makeFetchFallback(selection)(); + return makeFetchFallback(selection, { fresh: true })(); }, { preflightPassed: true }, ); @@ -251,10 +268,9 @@ export function useGsapAwareEditing({ try { await flushQueued(); } catch (error) { - const selection = updates.at(-1)?.selection; - if (selection) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - } + // The aggregate write has no uniquely failing member; do not misattribute + // its telemetry to whichever member happened to be last in the array. + trackGsapInteractionFailure(error, null, "drag", "Move animated layer (group)"); throw error; } }, diff --git a/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts b/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts index 80bef7f7c3..cce77f55b1 100644 --- a/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts +++ b/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts @@ -8,16 +8,16 @@ export function useGsapInteractionFailureTelemetry( showToast: (message: string, tone?: "error" | "info") => void, ) { return useCallback( - (error: unknown, selection: DomEditSelection, mutationType: string, label: string) => { + (error: unknown, selection: DomEditSelection | null, mutationType: string, label: string) => { trackStudioSaveFailure({ source: "gsap_commit", error, - filePath: selection.sourceFile ?? activeCompPath ?? "index.html", + filePath: selection?.sourceFile ?? activeCompPath ?? "index.html", mutationType, label, - targetId: selection.id, - targetSelector: selection.selector, - targetSourceFile: selection.sourceFile, + targetId: selection?.id, + targetSelector: selection?.selector, + targetSourceFile: selection?.sourceFile, }); showToast( isGsapEditBlockedError(error) ? error.message : "Failed to save animated edit.", diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 76ca2129da..940509f764 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -117,6 +117,31 @@ describe("applyPreviewSync", () => { expect(reloadPreview).not.toHaveBeenCalled(); }); + it("applies both plural and singular patches when a caller supplies both", () => { + patchRuntimeTweenInPlace.mockReturnValue(true); + + applyPreviewSync( + FAKE_IFRAME, + result(), + { + label: "mixed patch contract", + instantPatches: [ + { selector: "#group-a", change: { kind: "set" as const, props: { x: 1 } } }, + ], + instantPatch: { + selector: "#single-b", + change: { kind: "set" as const, props: { x: 2 } }, + }, + }, + vi.fn(), + ); + + expect(patchRuntimeTweenInPlace.mock.calls.map((call) => [call[1], call[4]])).toEqual([ + ["#group-a", true], + ["#single-b", false], + ]); + }); + it("instantPatches: one patch that misses falls the whole batch back to the reload", () => { patchRuntimeTweenInPlace.mockImplementation((_iframe, selector) => selector !== "#b"); applySoftReload.mockReturnValue("applied"); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index fb042945d6..640b5cfa8f 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -249,7 +249,10 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - const patches = options.instantPatches ?? (options.instantPatch ? [options.instantPatch] : []); + const patches = [ + ...(options.instantPatches ?? []), + ...(options.instantPatch ? [options.instantPatch] : []), + ]; if (patches.length > 0) { const deferSeek = options.deferPreviewSync === true; const missed = patches.find( @@ -370,7 +373,8 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra const instantPatches = calls .map(({ options: callOptions }) => callOptions.instantPatch) .filter((patch) => patch !== undefined); - await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, instantPatches.length > 0 ? { ...options, instantPatches } : options); + const { instantPatch: _instantPatch, ...batchOptions } = options; + await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, instantPatches.length > 0 ? { ...batchOptions, instantPatches } : batchOptions); }, [showToast, finalizeSuccessfulMutation]); // Every GSAP-script commit is a read-modify-write of one file. Overlapping diff --git a/packages/studio/src/utils/studioDebug.ts b/packages/studio/src/utils/studioDebug.ts index a30b8f9bbe..73b3833fe6 100644 --- a/packages/studio/src/utils/studioDebug.ts +++ b/packages/studio/src/utils/studioDebug.ts @@ -1,7 +1,7 @@ // Opt-in diagnostic channels — one per question worth tracing, all off by // default. Turn one on for the session with `localStorage.setItem("hf--debug", // "1")` and reload, then grep the console for `[hf-]`. -// Live channels: reload, select, drag, resize. +// Live channels: reload, select, drag, resize, commit. // // These exist because the interesting failures here are decisions, not crashes: // a preview that reloads when it should not, a shift-click that selects nothing. From dff382a2a849d4834a345373fca0bfe135902b56 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 14:03:07 -0400 Subject: [PATCH 4/4] fix(studio): carry deferred preview fallbacks --- .../studio/src/hooks/gsapScriptCommitTypes.ts | 2 + .../studio/src/hooks/useGsapAwareEditing.ts | 2 + .../src/hooks/useGsapScriptCommits.test.tsx | 92 +++++++++++++++++++ .../studio/src/hooks/useGsapScriptCommits.ts | 42 ++++++--- 4 files changed, 126 insertions(+), 12 deletions(-) diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index ed9f4a6ae4..85071ab6d9 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -32,6 +32,8 @@ export interface CommitMutationOptions { * error handling — a failed write still throws. */ deferPreviewSync?: boolean; + /** Shares an in-place patch miss with the final render of one multi-write action. */ + previewFallbackLatch?: { pending: boolean }; beforeReload?: () => void; /** * Serialize this commit against others sharing the same key. Used to chain diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 094afa0278..384527f5fb 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -179,11 +179,13 @@ export function useGsapAwareEditing({ // positions are already on screen, so holding the render until the last // member has been written costs nothing and never shows a half-moved group. let renderOnCommit = false; + const previewFallbackLatch = { pending: false }; const withGroupOptions = (options: CommitMutationOptions): CommitMutationOptions => ({ ...options, coalesceKey, coalesceMs: Number.POSITIVE_INFINITY, deferPreviewSync: !renderOnCommit, + previewFallbackLatch, }); // Every member writes the same file. Queue their mutations and send them as // ONE request instead of one round trip per member: the server reads, parses diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 940509f764..e9deae6ccf 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -169,6 +169,63 @@ describe("applyPreviewSync", () => { }); }); + it("carries a deferred patch miss into the final batch render", () => { + const previewFallbackLatch = { pending: false }; + applySoftReload.mockReturnValue("applied"); + const reloadPreview = vi.fn(); + patchRuntimeTweenInPlace.mockReturnValueOnce(false).mockReturnValueOnce(true); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Move animated layer (group)", + softReload: true, + deferPreviewSync: true, + previewFallbackLatch, + instantPatch: { selector: "#missed", change: { kind: "set", props: { x: 1 } } }, + }, + reloadPreview, + ); + + expect(previewFallbackLatch.pending).toBe(true); + expect(applySoftReload).not.toHaveBeenCalled(); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Move animated layer (group)", + softReload: true, + previewFallbackLatch, + instantPatch: { selector: "#final", change: { kind: "set", props: { x: 2 } } }, + }, + reloadPreview, + ); + + expect(previewFallbackLatch.pending).toBe(false); + expect(applySoftReload).toHaveBeenCalledTimes(1); + }); + + it("falls back immediately when a deferred patch miss has no final-render latch", () => { + patchRuntimeTweenInPlace.mockReturnValue(false); + applySoftReload.mockReturnValue("applied"); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Deferred standalone write", + softReload: true, + deferPreviewSync: true, + instantPatch: { selector: "#missed", change: { kind: "set", props: { x: 1 } } }, + }, + vi.fn(), + ); + + expect(applySoftReload).toHaveBeenCalledTimes(1); + }); + it("instantPatch + patch fails: falls back to the soft reload, passing onAsyncFailure", () => { patchRuntimeTweenInPlace.mockReturnValue(false); applySoftReload.mockReturnValue("applied"); @@ -439,6 +496,41 @@ describe("runCommit — instantPatch wiring", () => { expect(deps.reloadPreview).not.toHaveBeenCalled(); }); + it("no-op batch still applies every plural instant patch", async () => { + patchRuntimeTweenInPlace.mockReturnValue(true); + mockFetchResult({ changed: false }); + const deps = renderCommitHook(); + const batch = deps.api.commitMutation.batch; + if (!batch) throw new Error("batch capability missing"); + + await act(async () => { + await batch( + [ + { + selection, + mutation: { type: "update-property", property: "x", value: 10 }, + options: { + label: "Move layer", + instantPatch: { selector: "#a", change: { kind: "set", props: { x: 10 } } }, + }, + }, + { + selection: { ...selection, id: "b", selector: "#b" }, + mutation: { type: "update-property", property: "x", value: 20 }, + options: { + label: "Move layer", + instantPatch: { selector: "#b", change: { kind: "set", props: { x: 20 } } }, + }, + }, + ], + { label: "Move animated layer (group)" }, + ); + }); + + expect(patchRuntimeTweenInPlace.mock.calls.map((call) => call[1])).toEqual(["#a", "#b"]); + expect(deps.reloadPreview).not.toHaveBeenCalled(); + }); + it("no-op commit whose instant patch MISSES soft-reloads (never full-reloads)", async () => { // Server contract: gsap-mutations returns scriptText on EVERY response, // including changed:false — so the fallback re-runs the identical script diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index 640b5cfa8f..8a902ce97a 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -126,7 +126,10 @@ function finishUnchangedMutation( reloadPreview: () => void, ): boolean { if (result.changed !== false) return false; - if (!options.skipReload && options.instantPatch) { + if ( + !options.skipReload && + (instantPatchesFor(options).length > 0 || options.previewFallbackLatch?.pending) + ) { applyPreviewSync(iframe, result, options, reloadPreview); } return true; @@ -249,10 +252,8 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - const patches = [ - ...(options.instantPatches ?? []), - ...(options.instantPatch ? [options.instantPatch] : []), - ]; + const patches = instantPatchesFor(options); + let needsFallback = options.previewFallbackLatch?.pending === true; if (patches.length > 0) { const deferSeek = options.deferPreviewSync === true; const missed = patches.find( @@ -265,15 +266,23 @@ export function applyPreviewSync( deferSeek || index < patches.length - 1, ), ); - // Patched in place — elements are already correct on screen; no reload needed. - if (!missed) return; - // The instant path couldn't patch in place — record the fallback so we can - // track how often the fast path misses before the soft/full reload below. - trackStudioEvent("gsap_instant_patch_fallback", { selector: missed.selector }); - // Fall through to the soft/full reload path below. + if (missed) { + // The instant path couldn't patch in place — record the fallback so we can + // track how often the fast path misses before the soft/full reload below. + trackStudioEvent("gsap_instant_patch_fallback", { selector: missed.selector }); + needsFallback = true; + } + // Patched in place — elements are already correct on screen; no reload needed + // unless an earlier deferred batch left one member unpatched. + if (!needsFallback) return; } // Written, but the caller has more writes to make and will render after the last. - if (options.deferPreviewSync) return; + if (options.deferPreviewSync && options.previewFallbackLatch) { + options.previewFallbackLatch.pending = needsFallback; + return; + } + if (options.deferPreviewSync && !needsFallback) return; + if (options.previewFallbackLatch) options.previewFallbackLatch.pending = false; if (options.softReload && result.scriptText) { // A soft-reloadable edit escalates to a full iframe remount ONLY on the // PERMANENT "cannot-soft-reload" result (the preview is genuinely stale/ @@ -293,6 +302,15 @@ export function applyPreviewSync( } } +function instantPatchesFor( + options: CommitMutationOptions, +): NonNullable { + return [ + ...(options.instantPatches ?? []), + ...(options.instantPatch ? [options.instantPatch] : []), + ]; +} + // oxfmt-ignore // fallow-ignore-next-line complexity export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIframeRef, editHistory, domEditSaveTimestampRef, reloadPreview, onCacheInvalidate, onFileContentChanged, showToast, sdkSession, publishSdkSession, writeProjectFile, forceReloadSdkSession }: GsapScriptCommitsParams) {