diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index 731c41813f..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); @@ -524,6 +525,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 +554,7 @@ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): v } } +/** 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/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..85071ab6d9 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -22,6 +22,18 @@ 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; + /** 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 @@ -39,6 +51,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..05d5c95d4c --- /dev/null +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts @@ -0,0 +1,108 @@ +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); + }); + + 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 71e38bbb61..390208e328 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -42,7 +42,35 @@ 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, + 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(() => { + // 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; +} + +async function requestParsedAnimations( projectId: string, sourceFile: string, ): Promise { 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 0e84c15826..384527f5fb 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"; @@ -37,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[]; @@ -50,7 +66,7 @@ export interface UseGsapAwareEditingParams { ) => () => Promise; trackGsapInteractionFailure: ( error: unknown, - selection: DomEditSelection, + selection: DomEditSelection | null, mutationType: string, label: string, ) => void; @@ -155,19 +171,54 @@ 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}`; - const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => - gsapCommitMutation(selection, mutation, { - ...options, - coalesceKey, - coalesceMs: Number.POSITIVE_INFINITY, + // 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 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 + // 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 { + // 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. + const preflightResults = await Promise.allSettled( + updates.map(async ({ selection }) => { const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); preflightAnimations.set(selection, animations); const outcome = await tryGsapDragIntercept( @@ -180,12 +231,20 @@ export function useGsapAwareEditing({ { preflightOnly: true }, ); assertGsapEditPersisted(outcome); - } catch (error) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - throw error; - } + }), + ); + const preflightFailure = firstPreflightFailure(preflightResults, updates); + if (preflightFailure) { + trackGsapInteractionFailure( + preflightFailure.error, + preflightFailure.selection, + "drag", + "Move animated layer (group)", + ); + throw preflightFailure.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, @@ -193,7 +252,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, { fresh: true })(); + }, { preflightPassed: true }, ); assertGsapEditPersisted(outcome); @@ -202,6 +267,14 @@ export function useGsapAwareEditing({ throw error; } } + try { + await flushQueued(); + } catch (error) { + // 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; + } }, [gsapCommitMutation, previewIframeRef, makeFetchFallback, trackGsapInteractionFailure], ); 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 6236d032ab..e9deae6ccf 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -73,14 +73,159 @@ 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, + ); expect(applySoftReload).not.toHaveBeenCalled(); 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("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"); + 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("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"); @@ -338,10 +483,51 @@ 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, + ); + 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(); }); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index f1a21a3afe..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,19 +252,37 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - if (options.instantPatch) { - const patched = patchRuntimeTweenInPlace( - iframe, - options.instantPatch.selector, - options.instantPatch.change, + const patches = instantPatchesFor(options); + let needsFallback = options.previewFallbackLatch?.pending === true; + 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; - // 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 }); - // 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 && 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/ @@ -281,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) { @@ -356,7 +386,13 @@ 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); + 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.