Skip to content

Commit e1289f8

Browse files
committed
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.
1 parent 887780d commit e1289f8

7 files changed

Lines changed: 54 additions & 51 deletions

File tree

packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ export function clearKeyframeCacheForFile(sourceFile: string): void {
114114
}
115115
}
116116

117-
function elementCacheKeys(sourceFile: string, elementId: string): string[] {
117+
export function elementCacheKeys(sourceFile: string, elementId: string): string[] {
118118
return sourceFile === "index.html"
119119
? [`index.html#${elementId}`, elementId]
120120
: [`${sourceFile}#${elementId}`, `index.html#${elementId}`, elementId];

packages/studio/src/hooks/keyframeCacheAstLoad.ts

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ import { isStudioHoldSet } from "@hyperframes/core/gsap-parser";
88
import { usePlayerStore } from "../player/store/playerStore";
99
import {
1010
clearKeyframeCacheForFile,
11+
elementCacheKeys,
1112
writeGsapAnimationsForElement,
1213
} from "./gsapKeyframeCacheHelpers";
13-
import { toAbsoluteTime } from "./gsapShared";
14+
import { toClipKeyframes } from "./gsapShared";
1415
import {
1516
deduplicateKeyframes,
1617
isStaticPositionHold,
@@ -56,10 +57,35 @@ export function resolveSelectorElementIds(
5657
}
5758
return Array.from(ids);
5859
}
60+
/**
61+
* The slice of the parse response callers actually read. The endpoint returns
62+
* the full `ParsedGsap` (preamble/postamble and all), but nothing downstream of
63+
* this fetch touches the source-text fields, so the guard below only has to
64+
* vouch for what gets used.
65+
*/
66+
type ParsedGsapAnimations = Pick<
67+
ParsedGsap,
68+
"animations" | "multipleTimelines" | "unsupportedTimelinePattern"
69+
>;
70+
71+
/**
72+
* A proxy, an error page, or a stale server can answer 200 with something that
73+
* has no `animations` array — the case where the old blind cast crashed on
74+
* `.animations.filter`.
75+
*/
76+
function hasAnimations(value: unknown): value is ParsedGsapAnimations {
77+
return (
78+
typeof value === "object" &&
79+
value !== null &&
80+
"animations" in value &&
81+
Array.isArray(value.animations)
82+
);
83+
}
84+
5985
export async function fetchParsedAnimations(
6086
projectId: string,
6187
sourceFile: string,
62-
): Promise<ParsedGsap | null> {
88+
): Promise<ParsedGsapAnimations | null> {
6389
try {
6490
const res = await fetch(
6591
`/api/projects/${encodeURIComponent(projectId)}/gsap-animations/${encodeURIComponent(sourceFile)}`,
@@ -68,7 +94,8 @@ export async function fetchParsedAnimations(
6894
{ cache: "no-store" },
6995
);
7096
if (!res.ok) return null;
71-
const parsed = (await res.json()) as ParsedGsap;
97+
const parsed: unknown = await res.json();
98+
if (!hasAnimations(parsed)) return null;
7299
// Studio-emitted pre-keyframe hold `set`s are an internal runtime detail (they
73100
// hold an element's first keyframe before its tween). They must not surface as
74101
// user animations — otherwise they pollute the keyframe cache / timeline diamonds.
@@ -131,8 +158,6 @@ export async function populateKeyframeCacheFromAst(
131158
if (isStaticPositionHold(anim)) continue;
132159
const kfData = anim.keyframes ?? synthesizeFlatTweenKeyframes(anim);
133160
if (!kfData) continue;
134-
const tweenPos = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0);
135-
const tweenDur = anim.duration ?? 1;
136161
// Attribute the tween to every element it animates (handles class /
137162
// group / descendant selectors, not just `#id`).
138163
for (const id of resolveSelectorElementIds(anim.targetSelector, doc)) {
@@ -142,22 +167,7 @@ export async function populateKeyframeCacheFromAst(
142167
// below records, or expanded lanes have nothing to render.
143168
sourceByElement.set(id, [...(sourceByElement.get(id) ?? []), anim]);
144169
const { elStart, elDuration } = resolveClipTimingBasis(id, sf, elements, domClipChildren);
145-
const clipKeyframes = kfData.keyframes.map((kf) => {
146-
const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage);
147-
// 0.001% precision (see useGsapAnimationsForElement) so a beat-snapped
148-
// keyframe centers on the beat dot and both caches agree.
149-
const clipPct =
150-
elDuration > 0
151-
? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000
152-
: kf.percentage;
153-
return {
154-
...kf,
155-
percentage: clipPct,
156-
tweenPercentage: kf.percentage,
157-
propertyGroup: anim.propertyGroup,
158-
animationId: anim.id, // parity with other cache writers; inline ease needs it
159-
};
160-
});
170+
const clipKeyframes = toClipKeyframes(kfData.keyframes, anim, elStart, elDuration);
161171
const existing = mergedByElement.get(id);
162172
if (existing) {
163173
existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]);
@@ -167,9 +177,7 @@ export async function populateKeyframeCacheFromAst(
167177
}
168178
}
169179
for (const [id, kfData] of mergedByElement) {
170-
setKeyframeCache(`${sf}#${id}`, kfData);
171-
setKeyframeCache(id, kfData);
172-
if (sf !== "index.html") setKeyframeCache(`index.html#${id}`, kfData);
180+
for (const key of elementCacheKeys(sf, id)) setKeyframeCache(key, kfData);
173181
writeGsapAnimationsForElement(sf, id, sourceByElement.get(id));
174182
}
175183
}

packages/studio/src/hooks/useGsapAnimationFetchFallback.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export type ElementAnimationsOutcome =
3636
* so the caller can apply the right retry budget to each.
3737
*/
3838
export function selectElementAnimationsOrRetry(
39-
parsed: ParsedGsap | null,
39+
parsed: Pick<ParsedGsap, "animations"> | null,
4040
target: { id: string | null; selector: string | null },
4141
): ElementAnimationsOutcome {
4242
if (!parsed) return { kind: "fetch-error" };

packages/studio/src/player/components/Timeline.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -954,11 +954,13 @@ describe("getTimelinePlayheadLeft", () => {
954954

955955
describe("getTimelineCanvasHeight", () => {
956956
it("includes bottom scroll buffer below the last track", () => {
957-
expect(getTimelineCanvasHeight(3)).toBeGreaterThan(RULER_H + 3 * TRACK_H);
957+
expect(getTimelineCanvasHeight([TRACK_H, TRACK_H, TRACK_H])).toBeGreaterThan(
958+
RULER_H + 3 * TRACK_H,
959+
);
958960
});
959961

960962
it("still keeps ruler space when there are no tracks", () => {
961-
expect(getTimelineCanvasHeight(0)).toBeGreaterThan(24);
963+
expect(getTimelineCanvasHeight([])).toBeGreaterThan(24);
962964
});
963965
});
964966

packages/studio/src/player/components/timelineLayout.test.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import {
1414
resolveTimelineAssetDrop,
1515
} from "./timelineLayout";
1616

17+
/** N collapsed rows, the shape every caller passes when nothing is expanded. */
18+
const baseRows = (count: number) => Array.from({ length: count }, () => TRACK_H);
19+
1720
describe("variable timeline row geometry", () => {
1821
const tracks = [
1922
[{ clipId: "a", laneCount: 0 }],
@@ -23,7 +26,7 @@ describe("variable timeline row geometry", () => {
2326

2427
it("resolves every row to the base height when no clip is expanded", () => {
2528
expect(trackHeights(tracks)).toEqual([TRACK_H, TRACK_H, TRACK_H]);
26-
expect(trackHeights(3)).toEqual([TRACK_H, TRACK_H, TRACK_H]);
29+
expect(trackHeights([[], [], []])).toEqual([TRACK_H, TRACK_H, TRACK_H]);
2730
});
2831

2932
it("adds one lane height per lane on an expanded clip", () => {
@@ -82,7 +85,7 @@ describe("collapsed timeline row geometry characterization", () => {
8285
[3, 290],
8386
[5, 386],
8487
])("keeps the %i-track canvas height at %i", (trackCount, expectedHeight) => {
85-
expect(getTimelineCanvasHeight(trackCount)).toBe(expectedHeight);
88+
expect(getTimelineCanvasHeight(baseRows(trackCount))).toBe(expectedHeight);
8689
});
8790
});
8891

@@ -127,20 +130,16 @@ describe("track-area breathing pad y-math", () => {
127130

128131
describe("getTimelineCanvasHeight", () => {
129132
it("reserves ruler + top pad + lanes + bottom pad", () => {
130-
expect(getTimelineCanvasHeight(0)).toBe(RULER_H + TRACKS_TOP_PAD + TRACKS_BOTTOM_PAD);
131-
expect(getTimelineCanvasHeight(3)).toBe(
133+
expect(getTimelineCanvasHeight([])).toBe(RULER_H + TRACKS_TOP_PAD + TRACKS_BOTTOM_PAD);
134+
expect(getTimelineCanvasHeight(baseRows(3))).toBe(
132135
RULER_H + TRACKS_TOP_PAD + 3 * TRACK_H + TRACKS_BOTTOM_PAD,
133136
);
134137
});
135138

136-
it("clamps a negative track count to zero lanes", () => {
137-
expect(getTimelineCanvasHeight(-4)).toBe(RULER_H + TRACKS_TOP_PAD + TRACKS_BOTTOM_PAD);
138-
});
139-
140139
it("leaves room below the last lane for a drag-into-void new track", () => {
141140
// The gap below the final lane must be at least a full track height so a
142141
// clip can be dropped there to create a new bottom track.
143-
const oneLane = getTimelineCanvasHeight(1);
142+
const oneLane = getTimelineCanvasHeight(baseRows(1));
144143
const lastLaneBottom = getTimelineRowTop(0) + TRACK_H;
145144
expect(oneLane - lastLaneBottom).toBeGreaterThanOrEqual(TRACK_H);
146145
});
@@ -155,7 +154,7 @@ describe("track-area breathing pad y-math", () => {
155154
contentOrigin: GUTTER,
156155
pixelsPerSecond: 100,
157156
duration: 60,
158-
rowHeights: trackHeights(3),
157+
rowHeights: baseRows(3),
159158
trackOrder: [0, 1, 2],
160159
};
161160

packages/studio/src/player/components/timelineLayout.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,9 @@ type TimelineTrackHeightInput = readonly (readonly TimelineTrackHeightClip[])[];
6262
* the shared row height.
6363
*/
6464
export function trackHeights(
65-
tracks: number | TimelineTrackHeightInput,
65+
tracks: TimelineTrackHeightInput,
6666
expandedClipIds?: ReadonlySet<string>,
6767
): number[] {
68-
if (typeof tracks === "number") {
69-
return Array.from({ length: Math.max(0, Math.trunc(tracks)) }, () => TRACK_H);
70-
}
7168
return tracks.map((clips) => {
7269
let laneCount = 0;
7370
if (expandedClipIds) {
@@ -444,15 +441,11 @@ export function getTimelinePlayheadLeft(
444441
return contentOrigin + Math.max(0, time) * Math.max(0, pixelsPerSecond) - PLAYHEAD_HEAD_W / 2;
445442
}
446443

447-
export function getTimelineCanvasHeight(trackCountOrHeights: number | readonly number[]): number {
444+
export function getTimelineCanvasHeight(rowHeights: readonly number[]): number {
448445
// RULER_H + top pad + lanes + bottom pad. The old TIMELINE_SCROLL_BUFFER is
449446
// subsumed by TRACKS_BOTTOM_PAD (which is larger), so the drag-into-void space
450447
// below the last lane is real scrollable surface, not a hidden buffer.
451-
const heights =
452-
typeof trackCountOrHeights === "number"
453-
? trackHeights(trackCountOrHeights)
454-
: trackCountOrHeights;
455-
const rowsHeight = getTimelineRowOffsets(heights).at(-1) ?? 0;
448+
const rowsHeight = getTimelineRowOffsets(rowHeights).at(-1) ?? 0;
456449
return RULER_H + TRACKS_TOP_PAD + rowsHeight + TRACKS_BOTTOM_PAD;
457450
}
458451

packages/studio/src/player/components/useTimelineTrackLayout.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,10 @@ export function resolveTrackKeyframeClip(
3838
return key === selectedElementId || selectedElementIds.has(key);
3939
});
4040
if (selected) return selected;
41-
return [...keyframed].sort(
42-
(a, b) => (laneCounts.get(b.key ?? b.id) ?? 0) - (laneCounts.get(a.key ?? a.id) ?? 0),
43-
)[0]!;
41+
// Most lanes wins, first one on a tie (same as the old stable sort), but as a
42+
// reduce over the already non-empty list so there's no index to assert on.
43+
const lanesOf = (element: TimelineElement) => laneCounts.get(element.key ?? element.id) ?? 0;
44+
return keyframed.reduce((best, element) => (lanesOf(element) > lanesOf(best) ? element : best));
4445
}
4546

4647
/** Lanes per clip: the count of distinct property groups whose tween contributes

0 commit comments

Comments
 (0)