From 538264463cd8f13c6f1adf1a6beb1612ed407954 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 26 Jul 2026 01:47:04 +0200 Subject: [PATCH] refactor(studio): tighten the expanded-lane cache and height helpers Validate the parse response before reading `.animations` instead of casting the JSON blind, and narrow the fetch's return type to the slice callers read. Route the AST cache load through the shared clip-keyframe and cache-key helpers so it can't drift from the other writer. Drop the unused numeric track-count branches from `trackHeights`/`getTimelineCanvasHeight`, and pick the widest keyframed clip with a reduce so there's no index assertion. --- .../studio/src/hooks/keyframeCacheAstLoad.ts | 56 +++++++++++-------- .../hooks/useGsapAnimationFetchFallback.ts | 2 +- .../src/player/components/Timeline.test.ts | 6 +- .../player/components/timelineLayout.test.ts | 19 +++---- .../src/player/components/timelineLayout.ts | 13 +---- .../components/useTimelineTrackLayout.ts | 7 ++- 6 files changed, 53 insertions(+), 50 deletions(-) diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index 6eb90b517..19eaacdcf 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -8,9 +8,10 @@ import { isStudioHoldSet } from "@hyperframes/core/gsap-parser"; import { usePlayerStore } from "../player/store/playerStore"; import { clearKeyframeCacheForFile, + elementCacheKeys, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { toAbsoluteTime } from "./gsapShared"; +import { toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, isStaticPositionHold, @@ -56,10 +57,35 @@ export function resolveSelectorElementIds( } return Array.from(ids); } +/** + * The slice of the parse response callers actually read. The endpoint returns + * the full `ParsedGsap` (preamble/postamble and all), but nothing downstream of + * this fetch touches the source-text fields, so the guard below only has to + * vouch for what gets used. + */ +type ParsedGsapAnimations = Pick< + ParsedGsap, + "animations" | "multipleTimelines" | "unsupportedTimelinePattern" +>; + +/** + * A proxy, an error page, or a stale server can answer 200 with something that + * has no `animations` array — the case where the old blind cast crashed on + * `.animations.filter`. + */ +function hasAnimations(value: unknown): value is ParsedGsapAnimations { + return ( + typeof value === "object" && + value !== null && + "animations" in value && + Array.isArray(value.animations) + ); +} + export async function fetchParsedAnimations( projectId: string, sourceFile: string, -): Promise { +): Promise { try { const res = await fetch( `/api/projects/${encodeURIComponent(projectId)}/gsap-animations/${encodeURIComponent(sourceFile)}`, @@ -68,7 +94,8 @@ export async function fetchParsedAnimations( { cache: "no-store" }, ); if (!res.ok) return null; - const parsed = (await res.json()) as ParsedGsap; + const parsed: unknown = await res.json(); + if (!hasAnimations(parsed)) return null; // Studio-emitted pre-keyframe hold `set`s are an internal runtime detail (they // hold an element's first keyframe before its tween). They must not surface as // user animations — otherwise they pollute the keyframe cache / timeline diamonds. @@ -131,8 +158,6 @@ export async function populateKeyframeCacheFromAst( if (isStaticPositionHold(anim)) continue; const kfData = anim.keyframes ?? synthesizeFlatTweenKeyframes(anim); if (!kfData) continue; - const tweenPos = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); - const tweenDur = anim.duration ?? 1; // Attribute the tween to every element it animates (handles class / // group / descendant selectors, not just `#id`). for (const id of resolveSelectorElementIds(anim.targetSelector, doc)) { @@ -142,22 +167,7 @@ export async function populateKeyframeCacheFromAst( // below records, or expanded lanes have nothing to render. sourceByElement.set(id, [...(sourceByElement.get(id) ?? []), anim]); const { elStart, elDuration } = resolveClipTimingBasis(id, sf, elements, domClipChildren); - const clipKeyframes = kfData.keyframes.map((kf) => { - const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage); - // 0.001% precision (see useGsapAnimationsForElement) so a beat-snapped - // keyframe centers on the beat dot and both caches agree. - const clipPct = - elDuration > 0 - ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 - : kf.percentage; - return { - ...kf, - percentage: clipPct, - tweenPercentage: kf.percentage, - propertyGroup: anim.propertyGroup, - animationId: anim.id, // parity with other cache writers; inline ease needs it - }; - }); + const clipKeyframes = toClipKeyframes(kfData.keyframes, anim, elStart, elDuration); const existing = mergedByElement.get(id); if (existing) { existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]); @@ -167,9 +177,7 @@ export async function populateKeyframeCacheFromAst( } } for (const [id, kfData] of mergedByElement) { - setKeyframeCache(`${sf}#${id}`, kfData); - setKeyframeCache(id, kfData); - if (sf !== "index.html") setKeyframeCache(`index.html#${id}`, kfData); + for (const key of elementCacheKeys(sf, id)) setKeyframeCache(key, kfData); writeGsapAnimationsForElement(sf, id, sourceByElement.get(id)); } } diff --git a/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts b/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts index e1fcded7d..4e6f6ef9b 100644 --- a/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts +++ b/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts @@ -36,7 +36,7 @@ export type ElementAnimationsOutcome = * so the caller can apply the right retry budget to each. */ export function selectElementAnimationsOrRetry( - parsed: ParsedGsap | null, + parsed: Pick | null, target: { id: string | null; selector: string | null }, ): ElementAnimationsOutcome { if (!parsed) return { kind: "fetch-error" }; diff --git a/packages/studio/src/player/components/Timeline.test.ts b/packages/studio/src/player/components/Timeline.test.ts index b1978443d..81b095961 100644 --- a/packages/studio/src/player/components/Timeline.test.ts +++ b/packages/studio/src/player/components/Timeline.test.ts @@ -954,11 +954,13 @@ describe("getTimelinePlayheadLeft", () => { describe("getTimelineCanvasHeight", () => { it("includes bottom scroll buffer below the last track", () => { - expect(getTimelineCanvasHeight(3)).toBeGreaterThan(RULER_H + 3 * TRACK_H); + expect(getTimelineCanvasHeight([TRACK_H, TRACK_H, TRACK_H])).toBeGreaterThan( + RULER_H + 3 * TRACK_H, + ); }); it("still keeps ruler space when there are no tracks", () => { - expect(getTimelineCanvasHeight(0)).toBeGreaterThan(24); + expect(getTimelineCanvasHeight([])).toBeGreaterThan(24); }); }); diff --git a/packages/studio/src/player/components/timelineLayout.test.ts b/packages/studio/src/player/components/timelineLayout.test.ts index 250345fff..63f0c97fe 100644 --- a/packages/studio/src/player/components/timelineLayout.test.ts +++ b/packages/studio/src/player/components/timelineLayout.test.ts @@ -16,6 +16,9 @@ import { resolveTimelineAssetDrop, } from "./timelineLayout"; +/** N collapsed rows, the shape every caller passes when nothing is expanded. */ +const baseRows = (count: number) => Array.from({ length: count }, () => TRACK_H); + describe("variable timeline row geometry", () => { const tracks = [ [{ clipId: "a", laneCount: 0 }], @@ -25,7 +28,7 @@ describe("variable timeline row geometry", () => { it("resolves every row to the base height when no clip is expanded", () => { expect(trackHeights(tracks)).toEqual([TRACK_H, TRACK_H, TRACK_H]); - expect(trackHeights(3)).toEqual([TRACK_H, TRACK_H, TRACK_H]); + expect(trackHeights([[], [], []])).toEqual([TRACK_H, TRACK_H, TRACK_H]); }); it("adds one lane height per lane on an expanded clip", () => { @@ -84,7 +87,7 @@ describe("collapsed timeline row geometry characterization", () => { [3, 290], [5, 386], ])("keeps the %i-track canvas height at %i", (trackCount, expectedHeight) => { - expect(getTimelineCanvasHeight(trackCount)).toBe(expectedHeight); + expect(getTimelineCanvasHeight(baseRows(trackCount))).toBe(expectedHeight); }); }); @@ -129,20 +132,16 @@ describe("track-area breathing pad y-math", () => { describe("getTimelineCanvasHeight", () => { it("reserves ruler + top pad + lanes + bottom pad", () => { - expect(getTimelineCanvasHeight(0)).toBe(RULER_H + TRACKS_TOP_PAD + TRACKS_BOTTOM_PAD); - expect(getTimelineCanvasHeight(3)).toBe( + expect(getTimelineCanvasHeight([])).toBe(RULER_H + TRACKS_TOP_PAD + TRACKS_BOTTOM_PAD); + expect(getTimelineCanvasHeight(baseRows(3))).toBe( RULER_H + TRACKS_TOP_PAD + 3 * TRACK_H + TRACKS_BOTTOM_PAD, ); }); - it("clamps a negative track count to zero lanes", () => { - expect(getTimelineCanvasHeight(-4)).toBe(RULER_H + TRACKS_TOP_PAD + TRACKS_BOTTOM_PAD); - }); - it("leaves room below the last lane for a drag-into-void new track", () => { // The gap below the final lane must be at least a full track height so a // clip can be dropped there to create a new bottom track. - const oneLane = getTimelineCanvasHeight(1); + const oneLane = getTimelineCanvasHeight(baseRows(1)); const lastLaneBottom = getTimelineRowTop(0) + TRACK_H; expect(oneLane - lastLaneBottom).toBeGreaterThanOrEqual(TRACK_H); }); @@ -157,7 +156,7 @@ describe("track-area breathing pad y-math", () => { contentOrigin: GUTTER, pixelsPerSecond: 100, duration: 60, - rowHeights: trackHeights(3), + rowHeights: baseRows(3), trackOrder: [0, 1, 2], }; diff --git a/packages/studio/src/player/components/timelineLayout.ts b/packages/studio/src/player/components/timelineLayout.ts index 4b045f743..4db33c01c 100644 --- a/packages/studio/src/player/components/timelineLayout.ts +++ b/packages/studio/src/player/components/timelineLayout.ts @@ -55,12 +55,9 @@ type TimelineTrackHeightInput = readonly (readonly TimelineTrackHeightClip[])[]; * the shared row height. */ export function trackHeights( - tracks: number | TimelineTrackHeightInput, + tracks: TimelineTrackHeightInput, expandedClipIds?: ReadonlySet, ): number[] { - if (typeof tracks === "number") { - return Array.from({ length: Math.max(0, Math.trunc(tracks)) }, () => TRACK_H); - } return tracks.map((clips) => { let laneCount = 0; if (expandedClipIds) { @@ -473,15 +470,11 @@ export function getTimelineScrubTime(input: { return Math.max(0, Math.min(duration, x / pixelsPerSecond)); } -export function getTimelineCanvasHeight(trackCountOrHeights: number | readonly number[]): number { +export function getTimelineCanvasHeight(rowHeights: readonly number[]): number { // RULER_H + top pad + lanes + bottom pad. The old TIMELINE_SCROLL_BUFFER is // subsumed by TRACKS_BOTTOM_PAD (which is larger), so the drag-into-void space // below the last lane is real scrollable surface, not a hidden buffer. - const heights = - typeof trackCountOrHeights === "number" - ? trackHeights(trackCountOrHeights) - : trackCountOrHeights; - const rowsHeight = getTimelineRowOffsets(heights).at(-1) ?? 0; + const rowsHeight = getTimelineRowOffsets(rowHeights).at(-1) ?? 0; return RULER_H + TRACKS_TOP_PAD + rowsHeight + TRACKS_BOTTOM_PAD; } diff --git a/packages/studio/src/player/components/useTimelineTrackLayout.ts b/packages/studio/src/player/components/useTimelineTrackLayout.ts index af28964d1..4aad5a876 100644 --- a/packages/studio/src/player/components/useTimelineTrackLayout.ts +++ b/packages/studio/src/player/components/useTimelineTrackLayout.ts @@ -38,9 +38,10 @@ export function resolveTrackKeyframeClip( return key === selectedElementId || selectedElementIds.has(key); }); if (selected) return selected; - return [...keyframed].sort( - (a, b) => (laneCounts.get(b.key ?? b.id) ?? 0) - (laneCounts.get(a.key ?? a.id) ?? 0), - )[0]!; + // Most lanes wins, first one on a tie (same as the old stable sort), but as a + // reduce over the already non-empty list so there's no index to assert on. + const lanesOf = (element: TimelineElement) => laneCounts.get(element.key ?? element.id) ?? 0; + return keyframed.reduce((best, element) => (lanesOf(element) > lanesOf(best) ? element : best)); } /** Lanes per clip: the count of distinct property groups whose tween contributes