From 6b11d3743339c13424073fca63791d941d3fbbf6 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 20 Jul 2026 18:49:28 +0200 Subject: [PATCH] fix(studio): publish keyframe cache refresh atomically --- .../hooks/gsapKeyframeCacheHelpers.test.ts | 137 +++++++++----- .../src/hooks/gsapKeyframeCacheHelpers.ts | 168 +++++++++++++----- .../studio/src/hooks/keyframeCacheAstLoad.ts | 13 +- .../studio/src/hooks/useGsapTweenCache.ts | 65 ++++--- 4 files changed, 257 insertions(+), 126 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts index 9502479b5..10415f1f6 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts @@ -3,8 +3,8 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; import { clearKeyframeCacheForElement, - clearKeyframeCacheForFile, pruneKeyframeCacheToFiles, + replaceKeyframeCacheForFile, updateKeyframeCacheFromParsed, } from "./gsapKeyframeCacheHelpers"; @@ -70,48 +70,6 @@ describe("clearKeyframeCacheForElement", () => { }); }); -describe("clearKeyframeCacheForFile", () => { - it("clears the prefixed, fallback, and bare keys for every element of the file", () => { - seed("comp.html#a"); - seed("index.html#a"); - seed("a"); - seed("comp.html#b"); - seed("b"); - - clearKeyframeCacheForFile("comp.html"); - - for (const key of ["comp.html#a", "index.html#a", "a", "comp.html#b", "b"]) { - expect(cache().has(key)).toBe(false); - } - }); - - // 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", () => { - seed("comp.html#a"); - seed("a"); - seed("other.html#z"); - seed("z"); - - clearKeyframeCacheForFile("comp.html"); - - expect(cache().has("other.html#z")).toBe(true); - expect(cache().has("z")).toBe(true); - }); -}); - describe("pruneKeyframeCacheToFiles", () => { // Switching composition leaves the previous comp's elements cached with no // owner left to clear them: each file only ever clears its own entries. @@ -152,6 +110,99 @@ describe("pruneKeyframeCacheToFiles", () => { expect(cache().has("index.html#hero")).toBe(true); expect(cache().has("comp.html#a")).toBe(true); }); + + // The prune runs immediately before the atomic repopulate on every + // composition switch. One notification per stale element put every subscriber + // through a cache that was progressively emptier, which is the flash the + // atomic repopulate exists to avoid. + it("drops every stale file in one notification", () => { + seed("old.html#a"); + seed("old.html#b"); + seed("older.html#c"); + seed("kept.html#k"); + let notifications = 0; + const unsubscribe = usePlayerStore.subscribe(() => { + notifications += 1; + }); + + pruneKeyframeCacheToFiles(["kept.html"]); + unsubscribe(); + + expect(notifications).toBe(1); + expect([...cache().keys()]).toEqual(["kept.html#k"]); + }); + + // A prune that finds nothing stale must not hand subscribers a fresh Map + // identity: every keyframe consumer re-renders on cache identity alone. + it("does not publish when nothing is stale", () => { + seed("kept.html#k"); + const before = cache(); + let notifications = 0; + const unsubscribe = usePlayerStore.subscribe(() => { + notifications += 1; + }); + + pruneKeyframeCacheToFiles(["kept.html"]); + unsubscribe(); + + expect(notifications).toBe(0); + expect(cache()).toBe(before); + }); +}); + +describe("replaceKeyframeCacheForFile", () => { + it("publishes complete keyframe and animation maps in one notification", () => { + const staleEntry = entry(); + const otherEntry = entry(); + const staleAnimation = animWithKeyframes("stale"); + const freshAnimation = animWithKeyframes("fresh"); + usePlayerStore.setState({ + keyframeCache: new Map([ + ["scene.html#stale", staleEntry], + ["index.html#stale", staleEntry], + ["stale", staleEntry], + ["other.html#other", otherEntry], + ["other", otherEntry], + ]), + gsapAnimations: new Map([ + ["scene.html#stale", [staleAnimation]], + ["index.html#stale", [staleAnimation]], + ["stale", [staleAnimation]], + ["other.html#other", [staleAnimation]], + ["other", [staleAnimation]], + ]), + }); + const snapshots: Array<{ cacheKeys: string[]; animationKeys: string[] }> = []; + const unsubscribe = usePlayerStore.subscribe((state) => { + snapshots.push({ + cacheKeys: [...state.keyframeCache.keys()].sort(), + animationKeys: [...state.gsapAnimations.keys()].sort(), + }); + }); + + replaceKeyframeCacheForFile( + "scene.html", + new Map([["fresh", entry()]]), + new Map([["fresh", [freshAnimation]]]), + ); + unsubscribe(); + + expect(snapshots).toEqual([ + { + cacheKeys: ["fresh", "index.html#fresh", "other", "other.html#other", "scene.html#fresh"], + animationKeys: [ + "fresh", + "index.html#fresh", + "other", + "other.html#other", + "scene.html#fresh", + ], + }, + ]); + expect(usePlayerStore.getState().gsapAnimations.get("scene.html#fresh")).toEqual([ + freshAnimation, + ]); + }); }); describe("updateKeyframeCacheFromParsed", () => { diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index bed6c0f43..b1d0db3cf 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -11,6 +11,47 @@ import { type MergeableKeyframe, } from "./gsapTweenSynth"; +/** Both cache maps mid-edit, before the single store publish. */ +export interface KeyframeCacheDraft { + keyframeCache: Map; + gsapAnimations: Map; +} + +function sameEntries(before: ReadonlyMap, after: ReadonlyMap): boolean { + if (before.size !== after.size) return false; + for (const [key, value] of before) { + if (after.get(key) !== value) return false; + } + return true; +} + +/** + * Every multi-key cache writer publishes through here: clone both maps once, + * let the caller edit the drafts, publish once. The per-key store setters turned + * one logical refresh into N notifications, and every subscriber that rendered + * in between saw a cache that was half the old file and half the new one. The + * identity check keeps an edit that changed nothing from allocating fresh maps + * and re-rendering every subscriber, which is the guard the per-key setters used + * to carry individually. + */ +export function publishKeyframeCache(edit: (draft: KeyframeCacheDraft) => void): void { + const { keyframeCache, gsapAnimations } = usePlayerStore.getState(); + const draft: KeyframeCacheDraft = { + keyframeCache: new Map(keyframeCache), + gsapAnimations: new Map(gsapAnimations), + }; + edit(draft); + if ( + sameEntries(keyframeCache, draft.keyframeCache) && + sameEntries(gsapAnimations, draft.gsapAnimations) + ) + return; + usePlayerStore.setState({ + keyframeCache: draft.keyframeCache, + gsapAnimations: draft.gsapAnimations, + }); +} + export function updateKeyframeCacheFromParsed( animations: GsapAnimation[], targetPath: string, @@ -18,7 +59,7 @@ export function updateKeyframeCacheFromParsed( mutation: Record, doc?: Document | null, ): void { - const { setKeyframeCache, elements, domClipChildren } = usePlayerStore.getState(); + const { elements, domClipChildren } = usePlayerStore.getState(); const idsWithKeyframes = new Set(); // Attributed keyframes only: everything in here came from a parsed tween via // toClipKeyframes, so the merge can rely on the source identity. It widens @@ -70,15 +111,41 @@ export function updateKeyframeCacheFromParsed( } } } - for (const [id, entry] of merged) { - for (const key of elementCacheKeys(targetPath, id)) setKeyframeCache(key, entry); - writeGsapAnimationsForElement(targetPath, id, sourceAnimations.get(id)); - } const mutationSelector = (mutation as { targetSelector?: string }).targetSelector; const mutated = mutationSelector ? resolveSelectorElementIds(mutationSelector, doc) : []; const targetIds = mutated.length > 0 ? mutated : selectionId ? [selectionId] : []; - for (const targetId of targetIds) { - if (!idsWithKeyframes.has(targetId)) clearKeyframeCacheForElement(targetPath, targetId); + publishKeyframeCache((draft) => { + for (const [id, entry] of merged) { + writeElementIntoDraft(draft, targetPath, id, entry, sourceAnimations.get(id)); + } + for (const targetId of targetIds) { + if (!idsWithKeyframes.has(targetId)) deleteElementFromDraft(draft, targetPath, targetId); + } + }); +} + +function writeElementIntoDraft( + draft: KeyframeCacheDraft, + sourceFile: string, + elementId: string, + entry: KeyframeCacheEntry, + animations: GsapAnimation[] | undefined, +): void { + for (const key of elementCacheKeys(sourceFile, elementId)) { + draft.keyframeCache.set(key, entry); + if (animations) draft.gsapAnimations.set(key, animations); + else draft.gsapAnimations.delete(key); + } +} + +function deleteElementFromDraft( + draft: KeyframeCacheDraft, + sourceFile: string, + elementId: string, +): void { + for (const key of elementCacheKeys(sourceFile, elementId)) { + draft.keyframeCache.delete(key); + draft.gsapAnimations.delete(key); } } @@ -91,50 +158,37 @@ export function updateKeyframeCacheFromParsed( * preview overlay) fall back to the bare id when an element has no * source-prefixed key — so a clear that drops only the prefixed keys leaves the * bare entry behind and those readers keep showing keyframes the element no - * longer has. Each delete is guarded by `has` so an absent key doesn't allocate - * a new cache map and re-render every subscriber. + * longer has. publishKeyframeCache skips the store write entirely when none of + * the keys were present, so an absent element doesn't re-render every + * subscriber. */ export function clearKeyframeCacheForElement(sourceFile: string, elementId: string): void { - const { keyframeCache, setKeyframeCache, gsapAnimations, setGsapAnimations } = - usePlayerStore.getState(); - const keys = elementCacheKeys(sourceFile, elementId); - for (const key of keys) { - if (keyframeCache.has(key)) setKeyframeCache(key, undefined); - if (gsapAnimations.has(key)) setGsapAnimations(key, undefined); - } + publishKeyframeCache((draft) => deleteElementFromDraft(draft, sourceFile, elementId)); } -/** - * Clear every cached element of `sourceFile` before a full re-scan repopulates - * it. Only the file's OWN prefixed keys name the ids to clear: every write sets - * the prefixed key (see elementCacheKeys), so the file's elements are all - * reachable that way, and clearKeyframeCacheForElement then takes the - * 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 { - const { keyframeCache, gsapAnimations } = usePlayerStore.getState(); +function cachedElementIdsForFile( + sourceFile: string, + keyframeCache: ReadonlyMap, + gsapAnimations: ReadonlyMap, +): Set { const sfPrefix = `${sourceFile}#`; const ids = new Set(); for (const key of [...keyframeCache.keys(), ...gsapAnimations.keys()]) { if (!key.startsWith(sfPrefix)) continue; ids.add(key.slice(sfPrefix.length)); } - for (const id of ids) { - clearKeyframeCacheForElement(sourceFile, id); - } + return ids; } /** * Drop every cached element owned by a file that is no longer on screen. Each - * file only ever clears its OWN entries (see clearKeyframeCacheForFile), so + * file only ever writes its OWN prefixed entries (see elementCacheKeys), so * switching composition left the previous composition's elements cached forever - * — 240 entries per switch on a 120-clip comp, in both keyframeCache and - * gsapAnimations, with nothing to evict them. Called once before a re-scan, with - * the full set of files that scan covers. + * (240 entries per switch on a 120-clip comp, in both keyframeCache and + * gsapAnimations, with nothing to evict them). Called once before a re-scan, + * with the full set of files that scan covers, and it publishes once: the prune + * runs immediately before the atomic repopulate, so a per-element publish here + * would reintroduce the empty-cache flash that repopulate was made to avoid. */ export function pruneKeyframeCacheToFiles(files: readonly string[]): void { const keep = new Set(files); @@ -142,8 +196,8 @@ export function pruneKeyframeCacheToFiles(files: readonly string[]): void { const stale = new Map>(); for (const key of [...keyframeCache.keys(), ...gsapAnimations.keys()]) { const hash = key.indexOf("#"); - // Bare-id aliases carry no owner; clearKeyframeCacheForElement takes them - // with their prefixed key, so skipping them here loses nothing. + // Bare-id aliases carry no owner; deleteElementFromDraft takes them with + // their prefixed key, so skipping them here loses nothing. if (hash < 0) continue; const sourceFile = key.slice(0, hash); if (keep.has(sourceFile)) continue; @@ -151,9 +205,11 @@ export function pruneKeyframeCacheToFiles(files: readonly string[]): void { ids.add(key.slice(hash + 1)); stale.set(sourceFile, ids); } - for (const [sourceFile, ids] of stale) { - for (const id of ids) clearKeyframeCacheForElement(sourceFile, id); - } + publishKeyframeCache((draft) => { + for (const [sourceFile, ids] of stale) { + for (const id of ids) deleteElementFromDraft(draft, sourceFile, id); + } + }); } /** @@ -177,15 +233,37 @@ export function elementCacheKeys(sourceFile: string, elementId: string): string[ : [`${sourceFile}#${elementId}`, `index.html#${elementId}`, elementId]; } +/** Replace one file's complete cache snapshot with one atomic store publish. */ +export function replaceKeyframeCacheForFile( + sourceFile: string, + entries: ReadonlyMap, + animationsByElement: ReadonlyMap, +): void { + publishKeyframeCache((draft) => { + for (const id of cachedElementIdsForFile( + sourceFile, + draft.keyframeCache, + draft.gsapAnimations, + )) { + deleteElementFromDraft(draft, sourceFile, id); + } + for (const [id, entry] of entries) { + writeElementIntoDraft(draft, sourceFile, id, entry, animationsByElement.get(id)); + } + }); +} + export function writeGsapAnimationsForElement( sourceFile: string, elementId: string, animations: GsapAnimation[] | undefined, ): void { - const { setGsapAnimations } = usePlayerStore.getState(); - for (const key of elementCacheKeys(sourceFile, elementId)) { - setGsapAnimations(key, animations); - } + publishKeyframeCache((draft) => { + for (const key of elementCacheKeys(sourceFile, elementId)) { + if (animations) draft.gsapAnimations.set(key, animations); + else draft.gsapAnimations.delete(key); + } + }); } function buildCacheKey(sourceFile: string, elementId: string): string { diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index 9e20c6ec1..71e38bbb6 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -6,11 +6,7 @@ import type { GsapAnimation, GsapKeyframesData, ParsedGsap } from "@hyperframes/core/gsap-parser"; import { isStudioHoldSet } from "@hyperframes/core/gsap-parser"; import { usePlayerStore } from "../player/store/playerStore"; -import { - clearKeyframeCacheForFile, - elementCacheKeys, - writeGsapAnimationsForElement, -} from "./gsapKeyframeCacheHelpers"; +import { replaceKeyframeCacheForFile } from "./gsapKeyframeCacheHelpers"; import { resolveClipTimingBasis, resolveSelectorElementIds, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, @@ -81,8 +77,6 @@ export async function populateKeyframeCacheFromAst( ): Promise { const parsed = await fetchParsedAnimations(projectId, sf); if (!parsed) return; - const { setKeyframeCache } = usePlayerStore.getState(); - clearKeyframeCacheForFile(sf); const { elements, domClipChildren } = usePlayerStore.getState(); const mergedByElement = new Map>(); const sourceByElement = new Map(); @@ -109,8 +103,5 @@ export async function populateKeyframeCacheFromAst( } } } - for (const [id, kfData] of mergedByElement) { - for (const key of elementCacheKeys(sf, id)) setKeyframeCache(key, kfData); - writeGsapAnimationsForElement(sf, id, sourceByElement.get(id)); - } + replaceKeyframeCacheForFile(sf, mergedByElement, sourceByElement); } diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index 3c693bf9b..cc16648ef 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -5,6 +5,7 @@ import { readRuntimeKeyframes, scanAllRuntimeKeyframes } from "./gsapRuntimeBrid import { clearKeyframeCacheForElement, pruneKeyframeCacheToFiles, + publishKeyframeCache, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; import { resolveClipTimingBasis, toAbsoluteTime, toClipPercentage } from "./gsapShared"; @@ -313,11 +314,15 @@ export function useGsapAnimationsForElement( ...(ease ? { ease } : {}), ...(easeEach ? { easeEach } : {}), }; - const { setKeyframeCache } = usePlayerStore.getState(); - setKeyframeCache(`${sourceFile}#${elementId}`, merged); - // PropertyPanel reads the cache by bare elementId (without sourceFile prefix), - // so write a duplicate entry under the bare key for cross-component lookups. - setKeyframeCache(elementId, merged); + // PropertyPanel reads the cache by bare elementId (without sourceFile + // prefix), so the same entry is written under the bare key for + // cross-component lookups. Both keys land in one publish: a reader that woke + // between two separate writes saw the prefixed key updated and the bare one + // still stale. + publishKeyframeCache((draft) => { + draft.keyframeCache.set(`${sourceFile}#${elementId}`, merged); + draft.keyframeCache.set(elementId, merged); + }); // eslint-disable-next-line react-hooks/exhaustive-deps }, [elementId, sourceFile, animations, domClipChildrenKey]); @@ -416,29 +421,35 @@ export function usePopulateKeyframeCacheForFile( } const scanned = scanAllRuntimeKeyframes(iframe, clipById); if (scanned.size === 0) return false; - const { setKeyframeCache, keyframeCache } = usePlayerStore.getState(); - for (const [id, data] of scanned) { - const cacheKey = `${sf}#${id}`; - const fallbackKey = `index.html#${id}`; - const alreadyCached = - keyframeCache.has(cacheKey) || keyframeCache.has(fallbackKey) || keyframeCache.has(id); - if (alreadyCached) continue; - // Skip position-only set tweens from runtime too — same filter as AST path - const isPosOnly = - data.keyframes.length === 1 && - Object.keys(data.keyframes[0].properties).every((k) => k === "x" || k === "y"); - if (isPosOnly) { - continue; + // One publish for the whole scan: a scan of a 120-clip composition used to + // emit up to three store notifications per element, and every subscriber + // in between re-rendered against a cache only partly filled in. + publishKeyframeCache((draft) => { + for (const [id, data] of scanned) { + const cacheKey = `${sf}#${id}`; + const fallbackKey = `index.html#${id}`; + const alreadyCached = + draft.keyframeCache.has(cacheKey) || + draft.keyframeCache.has(fallbackKey) || + draft.keyframeCache.has(id); + if (alreadyCached) continue; + // Skip position-only set tweens from runtime too, same filter as AST path + const isPosOnly = + data.keyframes.length === 1 && + Object.keys(data.keyframes[0].properties).every((k) => k === "x" || k === "y"); + if (isPosOnly) { + continue; + } + const entry = { + format: "percentage" as const, + keyframes: data.keyframes, + ...(data.easeEach ? { easeEach: data.easeEach } : {}), + }; + draft.keyframeCache.set(cacheKey, entry); + if (sf !== "index.html") draft.keyframeCache.set(fallbackKey, entry); + draft.keyframeCache.set(id, entry); } - const entry = { - format: "percentage" as const, - keyframes: data.keyframes, - ...(data.easeEach ? { easeEach: data.easeEach } : {}), - }; - setKeyframeCache(cacheKey, entry); - if (sf !== "index.html") setKeyframeCache(fallbackKey, entry); - setKeyframeCache(id, entry); - } + }); runtimeScanDoneRef.current = `kf-cache:${projectId}:${sf}:${version}`; return true; };