fix(studio): soft-reload GSAP property edits, preserve shader cache#1129
Conversation
GSAP property value edits (opacity, x, scale, etc.) now update the live timeline inside the preview iframe without triggering a full iframe reload. This preserves the WebGL context and shader transition cache, eliminating the loading overlay that appeared on every property edit. Implementation: - New gsapSoftReload.ts: kills the old GSAP timeline, re-executes the updated script, calls __hfForceTimelineRebind(), and re-seeks to the current time. Falls back to full reload on failure. - useGsapScriptCommits: passes softReload: true for property value edits via the existing (previously unused) softReload flag on commitMutation. - hyper-shader.ts: exposes __hfSuppressSceneMutations on the window so the soft-reload can suppress the MutationObserver during re-execution. - hyper-shader.ts: getDocumentScriptSignature now excludes pure GSAP animation scripts from the cache key hash, so full reloads (undo, external changes) don't invalidate transition caches when only animation values changed.
…tion The new script ran in the same global scope as the old one, causing Identifier tl has already been declared errors from const/let re-declarations. Wrapping in an IIFE creates a new lexical scope. Also remove the old script element before inserting the new one.
…ML parsing The mutation API already has the extracted GSAP script text (newScript) after rewriting. Return it as scriptText in the response so applySoftReload receives the script directly instead of parsing HTML client-side. This avoids DOMParser compatibility issues across test environments and is more reliable than regex-based extraction.
The client's findGsapScriptElement only matched gsap.timeline and __timelines. The server's extractGsapScriptBlock also matches .to( and .set(. Aligned the client heuristic to prevent silent fallback to full reload for compositions that use tl.to() without gsap.timeline in the same script.
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed end-to-end. (Home flagged "no description" in the delegation, but Miguel did include one — clear summary, what-changed, what-stays-the-same, and a 6-item test plan. The brief wasn't current.)
Diagnosis is sharp: GSAP property edits triggered a full iframe reload → WebGL context recreate → shader cache flush → multi-second overlay. The fix is in three coordinated parts that all stay close to a single principle: don't bust the iframe when only a tween value changed.
Perf math (per the cost-out-perf discipline)
| Per edit | Old | New |
|---|---|---|
| Cost | ~2-3s (iframe rebuild + WebGL context recreate + shader recompile + scene init) | ~50ms (kill timelines + swap one script + reseek) |
| Cadence | Every commitMutation (150ms debounce) | Same |
| Per 5-min editor session w/ active scrubbing | ~2000 edits × 2-3s = 4000-6000s of overlay time spread across the session | ~2000 × 50ms = ~100s |
~40-60× per-edit improvement. For shader-heavy compositions this is genuinely the difference between "usable editor" and "unusable editor." Right call to invest in the soft-reload path.
Walked the three pieces
1. applySoftReload (gsapSoftReload.ts):
const doReload = () => {
// 1. Kill all old timelines + clear the registry
const timelines = win.__timelines;
if (timelines) {
for (const key of Object.keys(timelines)) {
try { timelines[key]?.kill?.(); } catch {}
delete timelines[key];
}
}
// 2. Remove the old script + insert a new IIFE-wrapped one
oldScriptEl.remove();
const newScript = doc.createElement("script");
newScript.textContent = `(function(){${scriptText}\n})();`;
doc.body.appendChild(newScript);
// 3. Force rebind + seek + manual-edits re-apply
win.__hfForceTimelineRebind?.();
win.__player?.seek?.(currentTime);
win.__hfStudioManualEditsApply?.();
};Ordering is correct: kill first (so the new tweens don't double-tween with old ones), then swap script (registers new tweens), then rebind/seek/manual-apply. The try { kill } catch {} per-timeline is the right defensive shape — one broken timeline shouldn't stop the rest from being killed.
The __hfSuppressSceneMutations wrapper around the whole sequence prevents the shader MutationObserver from marking transitions dirty during the swap. Cleanly threaded through hyper-shader.ts:2208.
2. Cache key filter (getDocumentScriptSignature):
function isGsapAnimationOnlyScript(text: string): boolean {
return (
(text.includes("gsap.timeline") || text.includes("__timelines")) &&
!text.includes("HyperShader") &&
!text.includes("hyper-shader") &&
!text.includes("hyperShader")
);
}Conservative direction: a script containing the "HyperShader" substring anywhere is NOT excluded, so cache busts still fire if the GSAP script also wires shaders. The substring match would mis-classify a script that mentions HyperShader in a comment or unrelated identifier — but the cost of that mis-classification is "an extra cache bust," not corruption. Right asymmetry for a perf optimization.
3. studio-api route update:
gsap-mutations/* route now returns scriptText in the response so the soft-reload has the new script content without re-fetching the file. Clean addition.
Test coverage
gsapSoftReload.test.ts has 6 tests covering the happy path + the four return-false branches (null iframe / empty scriptText / no gsap / no __hfForceTimelineRebind). Plus a positive test that __hfSuppressSceneMutations is invoked when available. Solid for the unit boundary.
Three edge cases worth noting (non-blocking)
A. Multiple inline GSAP scripts in one composition
findGsapScriptElement picks the first <script> containing gsap.timeline / __timelines / .to( / .set(. If a composition has two inline GSAP scripts (rare but legal for compositions with multiple registered timelines), only the first is replaced. The second persists with stale content.
The full timeline registry is wiped before the new script runs, so the persistent old script's tl.to(...) calls are now against killed timelines → no-op. The edit reaches disk correctly, just not the live iframe. On the next full reload (undo, comp change), the iframe rebuilds from disk source and is consistent.
So this is a transient inconsistency between live iframe and disk, not a data loss. Probably fine — single-GSAP-script-per-comp is the dominant pattern — but worth a test that asserts soft-reload returns false (forcing full reload) when more than one matching script is present.
B. Template-wrapped sub-composition scripts
Browser querySelectorAll("script:not([src])") does NOT descend into <template> content (template content lives in a separate DocumentFragment). Sub-composition GSAP scripts inside <template> markup are invisible to findGsapScriptElement → no match → applySoftReload returns false → full reload fires. Safe behavior — soft-reload optimization just doesn't engage for sub-comps. Worth a sentence in the docstring scoping the optimization to root-document GSAP scripts.
C. IIFE wrap changes scope semantics
newScript.textContent = `(function(){${scriptText}\n})();`;The wrap scopes const/let/var/function declarations to the IIFE. Compositions that rely on top-level declarations being globally accessible (rare, since most idiomatic compositions are already self-IIFE'd) would behave differently under soft-reload vs full-reload. The most common shape (const tl = ...; window.__timelines["root"] = tl;) works correctly because the registration goes through window directly. The edge case is compositions that do function onTransition() {...} at top level and reference it elsewhere — that would break under soft-reload.
Not blocking; documenting the constraint in a code comment near the wrap is the cheapest fix.
Severity discipline (per the retro pattern)
Walked the next-user-action from the broken state:
- Multi-second overlay on every property edit → user makes 10 scrub edits → 10 multi-second overlays → editor experience is unusable for shader-heavy comps
- No write-path corruption — edits do persist to disk correctly, the overlay just murders the iframe state every time
- Classification "UX/perf improvement" is correct; not under-graded
Verdict
Materially LGTM. Diagnosis correct, fix targeted, fallback-to-full-reload is safe, test coverage is solid for the unit boundary. Three minor edge-case docs/tests as non-blocking follow-ups: multi-GSAP-script behavior, template-wrapped sub-comp scope, IIFE-wrap scope constraint.
Holding the stamp — same protocol. James gates.
— Rames Jusso (hyperframes)
- Return false (fallback to full reload) when multiple GSAP scripts exist in the document, since it's ambiguous which one to replace - Add docstring scoping the optimization to root-document scripts (template-wrapped sub-compositions fall back to full reload) - Add code comment explaining the IIFE scope constraint - Add test for the multi-script guard
vanceingalls
left a comment
There was a problem hiding this comment.
Architecture is sound. The three-part design — (1) return scriptText from the mutation API instead of re-parsing client-side, (2) suppress MutationObserver during kill/re-execute/re-seek, (3) exclude pure GSAP scripts from the shader cache key — is the right decomposition. Fallback to full reload on any failure path is correct. __hfForceTimelineRebind / __hfSuppressSceneMutations globals are already registered by prior-commit runtime infrastructure, not invented here.
Important — cache-key exclusion is narrower than the script-detection heuristic
isGsapAnimationOnlyScript (used by getDocumentScriptSignature to exclude scripts from the cache key) checks gsap.timeline | __timelines, but findGsapScriptElement was widened in the latest commit to also match .to( and .set(. A composition that uses tl.to() without gsap.timeline in the same script will be correctly soft-reloaded (new logic), but won't be excluded from the cache key on full-reload paths. The failure mode is the cache busting unnecessarily on undo/external-change reloads for that subset — not a regression, but the perf win from the cache-key exclusion is narrower than implied. Consider bringing isGsapAnimationOnlyScript in sync with findGsapScriptElement's heuristic.
Important — sub-composition (template) gap
Server extractGsapScriptBlock explicitly walks <template> element scripts (you even have the comment explaining why linkedom doesn't descend into template content). Client findGsapScriptElement uses doc.querySelectorAll("script:not([src])"), which won't find scripts inside <template>. For sub-composition previews, applySoftReload returns false → silent fallback to full reload. Graceful degradation, so no correctness bug, but the PR title "preserve shader cache, no loading overlay" won't hold for sub-comp property edits. Worth at least documenting in a comment on findGsapScriptElement.
Nit — script-execution errors aren't caught by the outer try/catch
Appending <script> to the document runs synchronously; JS errors inside propagate via window.onerror, not through the surrounding try. If the server returned a script with a syntax error (unlikely — parseGsapScript validates before returning — but theoretically possible), applySoftReload returns true while the timeline is broken. A dev-mode console.warn on the onerror path would surface this.
Nit — no observability on the fallback path
If the soft-reload triggers the !applySoftReload → reloadPreview() fallback in prod, there's no signal. A console.warn("[gsap-soft-reload] falling back to full reload:", reason) would help diagnose regressions.
Nit — RefObject vs MutableRefObject inconsistency
previewIframeRef is React.MutableRefObject<HTMLIFrameElement | null> in UseDomEditSessionParams but React.RefObject<HTMLIFrameElement | null> in GsapScriptCommitsParams. Assignable in both directions for this use case, but worth standardizing on RefObject (the narrower, read-only shape) everywhere the ref isn't being written to.
Tests are solid for the boundary cases. The happy-dom environment correctly prevents actual script execution, so the "kills and re-executes" test verifies the DOM operation sequence without the side effects — acceptable scope for a unit test.
— Vai
isGsapAnimationOnlyScript now also matches .to( and .set( patterns, matching findGsapScriptElement. Scripts using only tl.to() without gsap.timeline were excluded from soft-reload but still busted the shader cache on full-reload paths (undo, external changes).
vanceingalls
left a comment
There was a problem hiding this comment.
Round 2 — both importants from Round 1 are addressed.
Cache-key asymmetry — fixed. isGsapAnimationOnlyScript in hyper-shader.ts now includes .to( and .set( alongside gsap.timeline / __timelines, bringing it in sync with isGsapScript in gsapSoftReload.ts. Scripts that used only tl.to() will now be excluded from the cache-key signature on full-reload paths, not just soft-reloaded.
<template> content invisibility — fixed. applySoftReload's docstring explicitly scopes the optimization to root-document GSAP scripts and documents the querySelectorAll / template boundary, so the fallback to full reload for sub-comp previews is intentional and communicated.
CI is green on 085fa54c. Approving.
— Vai
Summary
GSAP property value edits in the design panel now update the live timeline without reloading the preview iframe. This preserves the WebGL context and shader transition cache, eliminating the multi-second loading overlay that appeared on every property edit.
What changed
Soft-reload for property edits — after the GSAP mutation API writes the file, the studio kills the old timeline, re-executes the updated script inside the iframe, calls
__hfForceTimelineRebind(), and re-seeks to the current time. Falls back to full reload on failure.MutationObserver suppression — exposes
__hfSuppressSceneMutationsfrom hyper-shader.ts so the soft-reload can wrap the kill/re-execute/re-seek sequence without the observer marking transitions dirty.Smarter shader cache keys —
getDocumentScriptSignaturenow excludes pure GSAP animation scripts (those containinggsap.timeline/__timelinesbut NOTHyperShader.init). Full reloads (undo, external changes) no longer bust the transition cache when only animation values changed.What stays the same
softReloadflag oncommitMutationwas already in the type signature — this PR is the first callerTest plan