fix(studio): scope the per-file keyframe-cache clear to its own keys

R3 review follow-ups on the keyframe cache:

- clearKeyframeCacheForFile collected ids from the index.html alias prefix
  too, so a re-scan of one composition file wiped rows a sibling file had
  just written (several files re-scan concurrently). Only the file's own
  prefixed keys name the ids now; clearKeyframeCacheForElement still takes
  the alias and bare key with them.
- toClipKeyframes fell back to a fixed 1s tween duration, which put a
  duration-less tween's keyframes at a percentage no edit path agreed with.
  It now spans the clip, matching resolveEditableTweenDuration.
- collectAnimatableKeyframeProperties takes `object` so call sites drop
  their `as Record<string, unknown>` casts.

Regression tests cover both fixes.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-26 02:13:38 +02:00
parent acf6766ed8
commit d05ecb1091
4 changed files with 55 additions and 16 deletions
@@ -84,6 +84,20 @@ describe("clearKeyframeCacheForFile", () => {
} }
}); });
// Several composition files re-scan concurrently, so a clear that walked the
// index.html alias would delete rows a sibling file had just written.
it("leaves an index.html-owned element alone when another file re-scans", () => {
seed("index.html#title");
seed("title");
seed("comp.html#a");
clearKeyframeCacheForFile("comp.html");
expect(cache().has("index.html#title")).toBe(true);
expect(cache().has("title")).toBe(true);
expect(cache().has("comp.html#a")).toBe(false);
});
it("leaves entries that belong to a different source file", () => { it("leaves entries that belong to a different source file", () => {
seed("comp.html#a"); seed("comp.html#a");
seed("a"); seed("a");
@@ -92,22 +92,22 @@ export function clearKeyframeCacheForElement(sourceFile: string, elementId: stri
/** /**
* Clear every cached element of `sourceFile` before a full re-scan repopulates * Clear every cached element of `sourceFile` before a full re-scan repopulates
* it. Collects the element ids that currently have a prefixed or index.html * it. Only the file's OWN prefixed keys name the ids to clear: every write sets
* fallback key for the file and drops each through clearKeyframeCacheForElement * the prefixed key (see elementCacheKeys), so the file's elements are all
* so the bare key goes too — an element whose keyframes were removed (and so is * reachable that way, and clearKeyframeCacheForElement then takes the
* absent from the re-scan) leaves no stale bare entry behind. * index.html alias and the bare key with them — an element whose keyframes were
* removed (and so is absent from the re-scan) leaves no stale bare entry
* behind. Reading the alias prefix here instead would collect ids owned by
* OTHER files, and several files re-scan concurrently, so this file's clear
* would wipe the entries a sibling file had just written.
*/ */
export function clearKeyframeCacheForFile(sourceFile: string): void { export function clearKeyframeCacheForFile(sourceFile: string): void {
const { keyframeCache, gsapAnimations } = usePlayerStore.getState(); const { keyframeCache, gsapAnimations } = usePlayerStore.getState();
const sfPrefix = `${sourceFile}#`; const sfPrefix = `${sourceFile}#`;
const fallbackPrefix = "index.html#";
const ids = new Set<string>(); const ids = new Set<string>();
for (const key of [...keyframeCache.keys(), ...gsapAnimations.keys()]) { for (const key of [...keyframeCache.keys(), ...gsapAnimations.keys()]) {
const matchesFile = if (!key.startsWith(sfPrefix)) continue;
key.startsWith(sfPrefix) || (sourceFile !== "index.html" && key.startsWith(fallbackPrefix)); ids.add(key.slice(sfPrefix.length));
if (!matchesFile) continue;
const hashIdx = key.indexOf("#");
if (hashIdx !== -1) ids.add(key.slice(hashIdx + 1));
} }
for (const id of ids) { for (const id of ids) {
clearKeyframeCacheForElement(sourceFile, id); clearKeyframeCacheForElement(sourceFile, id);
@@ -4,6 +4,7 @@ import {
idSelector, idSelector,
isInstantHold, isInstantHold,
parsePercentageKeyframes, parsePercentageKeyframes,
toClipKeyframes,
toClipPercentage, toClipPercentage,
} from "./gsapShared"; } from "./gsapShared";
@@ -122,3 +123,26 @@ describe("toClipPercentage", () => {
expect(toClipPercentage(5, 0, 0, 42)).toBe(42); expect(toClipPercentage(5, 0, 0, 42)).toBe(42);
}); });
}); });
describe("toClipKeyframes", () => {
const durationless: GsapAnimation = {
id: "a1",
method: "to",
targetSelector: "#box",
vars: {},
resolvedStart: 0,
} as GsapAnimation;
// A tween with no duration spans its clip everywhere else in Studio
// (resolveEditableTweenDuration), so the cache rows have to agree: a fixed 1s
// basis put the end keyframe at 25% of a 4s clip instead of 100%.
it("spans the clip when the tween has no duration", () => {
const rows = toClipKeyframes([{ percentage: 0 }, { percentage: 100 }], durationless, 0, 4);
expect(rows.map((row) => row.percentage)).toEqual([0, 100]);
});
it("keeps the tween percentage and the animation identity on every row", () => {
const rows = toClipKeyframes([{ percentage: 50 }], durationless, 0, 4);
expect(rows[0]).toMatchObject({ tweenPercentage: 50, animationId: "a1" });
});
});
+7 -6
View File
@@ -145,9 +145,7 @@ export interface ParsedPercentageKeyframes {
easeEach?: string; easeEach?: string;
} }
function collectAnimatableKeyframeProperties( function collectAnimatableKeyframeProperties(entry: object): Record<string, number | string> {
entry: Record<string, unknown>,
): Record<string, number | string> {
const properties: Record<string, number | string> = {}; const properties: Record<string, number | string> = {};
for (const [property, value] of Object.entries(entry)) { for (const [property, value] of Object.entries(entry)) {
if (property === "ease") continue; if (property === "ease") continue;
@@ -185,7 +183,7 @@ export function parsePercentageKeyframes(
steps.forEach((entry, i) => { steps.forEach((entry, i) => {
if (!entry || typeof entry !== "object") return; if (!entry || typeof entry !== "object") return;
const percentage = steps.length > 1 ? Math.round((i / (steps.length - 1)) * 1000) / 10 : 0; const percentage = steps.length > 1 ? Math.round((i / (steps.length - 1)) * 1000) / 10 : 0;
const properties = collectAnimatableKeyframeProperties(entry as Record<string, unknown>); const properties = collectAnimatableKeyframeProperties(entry);
if (Object.keys(properties).length > 0) keyframes.push({ percentage, properties }); if (Object.keys(properties).length > 0) keyframes.push({ percentage, properties });
}); });
return keyframes.length > 0 ? { keyframes } : null; return keyframes.length > 0 ? { keyframes } : null;
@@ -199,7 +197,7 @@ export function parsePercentageKeyframes(
const pctMatch = key.match(/^(\d+(?:\.\d+)?)%$/); const pctMatch = key.match(/^(\d+(?:\.\d+)?)%$/);
if (!pctMatch || !val || typeof val !== "object") continue; if (!pctMatch || !val || typeof val !== "object") continue;
const percentage = parseFloat(pctMatch[1]); const percentage = parseFloat(pctMatch[1]);
const properties = collectAnimatableKeyframeProperties(val as Record<string, unknown>); const properties = collectAnimatableKeyframeProperties(val);
if (Object.keys(properties).length > 0) { if (Object.keys(properties).length > 0) {
keyframes.push({ percentage, properties }); keyframes.push({ percentage, properties });
} }
@@ -253,7 +251,10 @@ export function toClipKeyframes<T extends { percentage: number }>(
} }
> { > {
const tweenStart = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); const tweenStart = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0);
const tweenDuration = anim.duration ?? 1; // A duration-less tween spans the clip, the same rule the edit paths use
// (resolveEditableTweenDuration). A fixed 1s here put its keyframes at a
// percentage no editor agreed with.
const tweenDuration = anim.duration ?? clipDuration;
return source.map((keyframe) => ({ return source.map((keyframe) => ({
...keyframe, ...keyframe,
percentage: toClipPercentage( percentage: toClipPercentage(