From 076c656d6e99c954604e7679293ec33a97e21ad8 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 9 Jul 2026 17:28:37 -0400 Subject: [PATCH] fix(studio): fold GSAP timing rewrites into the recorded history entry A timeline move/resize recorded the timing patch, then a server GSAP rewrite mutated the same file afterward, leaving the recorded after stale so an undo hit a hash conflict. The GSAP mutation now snapshots the touched files and records a follow-up edit under the same coalesceKey, with a per-entry coalesceMs override large enough to survive the GSAP round trip, so undo restores the original in one step. Applies to single-clip and group edits. --- .../src/hooks/timelineEditingHelpers.ts | 102 ++++++++++++++++++ .../studio/src/hooks/useTimelineEditing.ts | 36 ++++--- .../src/hooks/useTimelineGroupEditing.ts | 91 +++++++++------- packages/studio/src/utils/editHistory.test.ts | 61 +++++++++++ packages/studio/src/utils/editHistory.ts | 8 +- 5 files changed, 245 insertions(+), 53 deletions(-) diff --git a/packages/studio/src/hooks/timelineEditingHelpers.ts b/packages/studio/src/hooks/timelineEditingHelpers.ts index d266012a2..28d2e09ec 100644 --- a/packages/studio/src/hooks/timelineEditingHelpers.ts +++ b/packages/studio/src/hooks/timelineEditingHelpers.ts @@ -123,6 +123,8 @@ export interface RecordEditInput { label: string; kind: EditHistoryKind; coalesceKey?: string; + /** Per-entry coalesce window override (ms); lets a slow follow-up still merge. */ + coalesceMs?: number; files: Record; } @@ -430,6 +432,53 @@ export async function finishTimelineTimingFallback(input: { input.reloadPreview(); } +// Coalesce window for folding a GSAP mutation into the preceding timing edit; only has to +// outlast one GSAP server round-trip, never a real second edit. +const GSAP_HISTORY_COALESCE_MS = 10_000; + +/** + * A server GSAP rewrite mutates the same file the timing patch just wrote, but AFTER the + * timing edit was recorded, leaving the recorded `after` stale so an undo hits a hash + * conflict. This snapshots every touched file, runs the mutation, then records a follow-up + * edit under the same coalesceKey with a window wide enough to survive the GSAP round-trip, + * folding both writes into one undo step. Returns the mutation status for caller reloads. + */ +export async function foldGsapMutationIntoHistory(input: { + projectId: string; + paths: string[]; + label: string; + coalesceKey?: string; + recordEdit: (edit: RecordEditInput) => Promise; + gsapMutation: () => Promise; +}): Promise { + const uniquePaths = [...new Set(input.paths)]; + const before = new Map(); + for (const path of uniquePaths) { + before.set(path, await readFileContent(input.projectId, path)); + } + const status = await input.gsapMutation(); + if (status.mutated) { + const files: Record = {}; + for (const path of uniquePaths) { + const priorContent = before.get(path); + const finalContent = await readFileContent(input.projectId, path); + if (priorContent !== undefined && finalContent !== priorContent) { + files[path] = { before: priorContent, after: finalContent }; + } + } + if (Object.keys(files).length > 0) { + await input.recordEdit({ + label: input.label, + kind: "timeline", + coalesceKey: input.coalesceKey, + coalesceMs: GSAP_HISTORY_COALESCE_MS, + files, + }); + } + } + return status; +} + /** * Shift all GSAP animation positions targeting a given element by a time delta. * Calls the server-side GSAP mutation endpoint which uses the AST-based parser. @@ -493,5 +542,58 @@ export async function scaleGsapPositions( return readMutationStatus(await res.json().catch(() => null)); } +/** Single-clip move GSAP shift, folded into the timing edit's history entry (see above). */ +export function foldedShiftGsapMutation(input: { + projectId: string; + targetPath: string; + domId: string; + delta: number; + label: string; + coalesceKey?: string; + recordEdit: (edit: RecordEditInput) => Promise; +}): () => Promise { + return () => + foldGsapMutationIntoHistory({ + projectId: input.projectId, + paths: [input.targetPath], + label: input.label, + coalesceKey: input.coalesceKey, + recordEdit: input.recordEdit, + gsapMutation: () => + shiftGsapPositions(input.projectId, input.targetPath, input.domId, input.delta), + }); +} + +/** Single-clip resize GSAP scale, folded into the timing edit's history entry (see above). */ +export function foldedScaleGsapMutation(input: { + projectId: string; + targetPath: string; + domId: string; + from: { start: number; duration: number }; + to: { start: number; duration: number }; + label: string; + coalesceKey?: string; + recordEdit: (edit: RecordEditInput) => Promise; +}): () => Promise { + return () => + foldGsapMutationIntoHistory({ + projectId: input.projectId, + paths: [input.targetPath], + label: input.label, + coalesceKey: input.coalesceKey, + recordEdit: input.recordEdit, + gsapMutation: () => + scaleGsapPositions( + input.projectId, + input.targetPath, + input.domId, + input.from.start, + input.from.duration, + input.to.start, + input.to.duration, + ), + }); +} + // Re-export applyPatchByTarget for use in the hook (avoids double import in callers) export { applyPatchByTarget, formatTimelineAttributeNumber }; diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index 0f78896bb..b873f8517 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -24,9 +24,9 @@ import { patchIframeDomTiming, persistTimelineEdit, readFileContent, + foldedShiftGsapMutation, + foldedScaleGsapMutation, formatTimelineAttributeNumber, - shiftGsapPositions, - scaleGsapPositions, finishTimelineTimingFallback, extendRootDurationIfNeeded, buildTimelineMoveTimingPatch, @@ -132,7 +132,6 @@ export function useTimelineEditing({ writeProjectFile, }); - // fallow-ignore-next-line complexity const handleTimelineElementMove = useCallback( // fallow-ignore-next-line complexity (element: TimelineElement, updates: TimelineMoveUpdates) => { @@ -172,7 +171,15 @@ export function useTimelineEditing({ reloadPreview, gsapMutation: delta !== 0 && domId && pid - ? () => shiftGsapPositions(pid, targetPath, domId, delta) + ? foldedShiftGsapMutation({ + projectId: pid, + targetPath, + domId, + delta, + label: "Move timeline clip", + coalesceKey, + recordEdit, + }) : undefined, onGsapError: (err) => console.error("[Timeline] Failed to shift GSAP positions", err), }); @@ -217,7 +224,6 @@ export function useTimelineEditing({ ], ); - // fallow-ignore-next-line complexity const handleTimelineElementResize = useCallback( // fallow-ignore-next-line complexity ( @@ -264,16 +270,16 @@ export function useTimelineEditing({ reloadPreview, gsapMutation: timingChanged && domId && pid - ? () => - scaleGsapPositions( - pid, - targetPath, - domId, - element.start, - element.duration, - updates.start, - updates.duration, - ) + ? foldedScaleGsapMutation({ + projectId: pid, + targetPath, + domId, + from: { start: element.start, duration: element.duration }, + to: { start: updates.start, duration: updates.duration }, + label: "Resize timeline clip", + coalesceKey, + recordEdit, + }) : undefined, onGsapError: (err) => console.error("[Timeline] Failed to scale GSAP positions", err), }); diff --git a/packages/studio/src/hooks/useTimelineGroupEditing.ts b/packages/studio/src/hooks/useTimelineGroupEditing.ts index ab67deac3..0ad9d365d 100644 --- a/packages/studio/src/hooks/useTimelineGroupEditing.ts +++ b/packages/studio/src/hooks/useTimelineGroupEditing.ts @@ -7,6 +7,7 @@ import { buildTimelineResizeTimingPatch, extendRootDurationIfNeeded, finishTimelineTimingFallback, + foldGsapMutationIntoHistory, formatTimelineAttributeNumber, patchIframeDomTiming, persistTimelineBatchEdit, @@ -202,22 +203,30 @@ export function useTimelineGroupEditing({ needsExtension, rootDurationSeconds: maxEnd, reloadPreview, - gsapMutation: async () => { - let mutated = false; - for (const change of changes) { - const delta = change.start - change.element.start; - const domId = change.element.domId; - if (delta === 0 || !domId) continue; - const status = await shiftGsapPositions( - projectId, - targetPathFor(change.element, activeCompPath), - domId, - delta, - ); - mutated = mutated || status.mutated; - } - return { mutated }; - }, + gsapMutation: () => + foldGsapMutationIntoHistory({ + projectId, + paths: changes.map((change) => targetPathFor(change.element, activeCompPath)), + label: "Move timeline clips", + coalesceKey, + recordEdit, + gsapMutation: async () => { + let mutated = false; + for (const change of changes) { + const delta = change.start - change.element.start; + const domId = change.element.domId; + if (delta === 0 || !domId) continue; + const status = await shiftGsapPositions( + projectId, + targetPathFor(change.element, activeCompPath), + domId, + delta, + ); + mutated = mutated || status.mutated; + } + return { mutated }; + }, + }), onGsapError: (err) => console.error("[Timeline] Failed to shift GSAP positions", err), }); }); @@ -310,27 +319,35 @@ export function useTimelineGroupEditing({ needsExtension, rootDurationSeconds: maxEnd, reloadPreview, - gsapMutation: async () => { - let mutated = false; - for (const change of changes) { - const domId = change.element.domId; - const timingChanged = - change.start !== change.element.start || - change.duration !== change.element.duration; - if (!timingChanged || !domId) continue; - const status = await scaleGsapPositions( - projectId, - targetPathFor(change.element, activeCompPath), - domId, - change.element.start, - change.element.duration, - change.start, - change.duration, - ); - mutated = mutated || status.mutated; - } - return { mutated }; - }, + gsapMutation: () => + foldGsapMutationIntoHistory({ + projectId, + paths: changes.map((change) => targetPathFor(change.element, activeCompPath)), + label: "Resize timeline clips", + coalesceKey, + recordEdit, + gsapMutation: async () => { + let mutated = false; + for (const change of changes) { + const domId = change.element.domId; + const timingChanged = + change.start !== change.element.start || + change.duration !== change.element.duration; + if (!timingChanged || !domId) continue; + const status = await scaleGsapPositions( + projectId, + targetPathFor(change.element, activeCompPath), + domId, + change.element.start, + change.element.duration, + change.start, + change.duration, + ); + mutated = mutated || status.mutated; + } + return { mutated }; + }, + }), onGsapError: (err) => console.error("[Timeline] Failed to scale GSAP positions", err), }); }); diff --git a/packages/studio/src/utils/editHistory.test.ts b/packages/studio/src/utils/editHistory.test.ts index 193bd05fe..b2642e0f1 100644 --- a/packages/studio/src/utils/editHistory.test.ts +++ b/packages/studio/src/utils/editHistory.test.ts @@ -209,6 +209,67 @@ describe("edit history", () => { expect(state.undo[0].files["index.html"].after).toBe("c"); }); + it("folds a slow GSAP follow-up into the timing edit via a per-entry coalesceMs override", () => { + const timing = buildEditHistoryEntry({ + projectId: "project-1", + label: "Resize timeline clip", + kind: "timeline", + coalesceKey: "timeline-resize:clip", + files: { "index.html": { before: "orig", after: "timing" } }, + now: 0, + id: "timing", + }); + // The server GSAP rewrite lands ~2s later, past the 300ms default window, but the + // follow-up carries a large coalesceMs so undo still collapses to a single step. + const gsap = buildEditHistoryEntry({ + projectId: "project-1", + label: "Resize timeline clip", + kind: "timeline", + coalesceKey: "timeline-resize:clip", + coalesceMs: 10_000, + files: { "index.html": { before: "timing", after: "timing+gsap" } }, + now: 2000, + id: "gsap", + }); + + const state = pushEditHistoryEntry( + pushEditHistoryEntry(createEmptyEditHistory(), timing), + gsap, + ); + + expect(state.undo).toHaveLength(1); + expect(state.undo[0].files["index.html"].before).toBe("orig"); + expect(state.undo[0].files["index.html"].after).toBe("timing+gsap"); + }); + + it("does not merge a slow follow-up without the coalesceMs override", () => { + const timing = buildEditHistoryEntry({ + projectId: "project-1", + label: "Resize timeline clip", + kind: "timeline", + coalesceKey: "timeline-resize:clip", + files: { "index.html": { before: "orig", after: "timing" } }, + now: 0, + id: "timing", + }); + const late = buildEditHistoryEntry({ + projectId: "project-1", + label: "Resize timeline clip", + kind: "timeline", + coalesceKey: "timeline-resize:clip", + files: { "index.html": { before: "timing", after: "timing+gsap" } }, + now: 2000, + id: "late", + }); + + const state = pushEditHistoryEntry( + pushEditHistoryEntry(createEmptyEditHistory(), timing), + late, + ); + + expect(state.undo).toHaveLength(2); + }); + it("coalesces entries with the same coalesceKey within the window (prop: format)", () => { const first = buildEditHistoryEntry({ projectId: "project-1", diff --git a/packages/studio/src/utils/editHistory.ts b/packages/studio/src/utils/editHistory.ts index f10474b4b..a5f938ce5 100644 --- a/packages/studio/src/utils/editHistory.ts +++ b/packages/studio/src/utils/editHistory.ts @@ -13,6 +13,8 @@ export interface EditHistoryEntry { label: string; kind: EditHistoryKind; coalesceKey?: string; + /** Per-entry coalesce window override (ms). Falls back to the reducer default. */ + coalesceMs?: number; createdAt: number; files: Record; } @@ -35,6 +37,7 @@ export interface BuildEditHistoryEntryInput { label: string; kind?: EditHistoryKind; coalesceKey?: string; + coalesceMs?: number; now: number; files: Record; } @@ -99,6 +102,7 @@ export function buildEditHistoryEntry(input: BuildEditHistoryEntryInput): EditHi label: input.label, kind: input.kind ?? "manual", coalesceKey: input.coalesceKey, + coalesceMs: input.coalesceMs, createdAt: input.now, files, }; @@ -111,7 +115,9 @@ export function pushEditHistoryEntry( ): EditHistoryState { if (Object.keys(entry.files).length === 0) return state; - const coalesceMs = options?.coalesceMs ?? DEFAULT_COALESCE_MS; + // The incoming entry's own window wins so a caller can guarantee a merge even when a + // slow async step (e.g. a server GSAP rewrite) sits between the two records. + const coalesceMs = entry.coalesceMs ?? options?.coalesceMs ?? DEFAULT_COALESCE_MS; const maxEntries = options?.maxEntries ?? DEFAULT_MAX_ENTRIES; const previous = state.undo[state.undo.length - 1]; let undo = state.undo;