fix(studio): a resize after a drag saves its size below the timeline it uses - #4638
Conversation
…line it uses A canvas drag stores the element position as a global gsap.set above the timeline declaration. A resize that followed it inserted its tl.set right after that line, so the saved script referenced the timeline before it was declared and every reload and render threw a ReferenceError. Both GSAP writers now bound the "after the last statement" anchor by the timeline declaration, whatever the timeline variable is named. A drag or a resize on its own writes the same bytes as before.
somanshreddy
left a comment
There was a problem hiding this comment.
Reviewed at 5c1d1f1ceea62da421e339dc103cde49d9ed82c1. Approving. Both writers are independently covered, and the fix can't reorder existing timeline calls — which is the thing I'd worry about with a change that moves an insertion anchor.
The symptom is real, reproduced end to end
Worth confirming the user-visible claim rather than just the byte offset, since the whole point is "every load and render threw":
BROKEN order -> ReferenceError: Cannot access 'tl' before initialization
FIXED order -> ran clean
That's the exact TDZ error, from the exact statement ordering in the description. A saved film with a tl.set above const tl is dead on load — not degraded, dead — so this is a real severity, not a tidiness fix.
Both writers are independently pinned
The repo has two GSAP script writers and the bug was in both, so the interesting question is whether the tests actually hold both ends. I reverted each fix on its own:
revert ACORN only (drop the Math.max clamp) 3 failed | 5 passed
× drag then resize with `const tl`
× drag then resize with `const master`
× drag then a keyframed tween
revert RECAST only (anchor always on lastPath) 3 failed | 5 passed
(same three)
restored 8 passed
Neither writer is riding on the other's coverage. The const master case also confirms the claim that the timeline variable name is read from the script rather than assumed to be tl.
The fix can't reorder existing timeline calls
This is the part I went looking for a bug in and didn't find. Moving an insertion anchor later in the file is exactly how you'd accidentally change GSAP semantics: timeline calls carry position parameters, and same-position ties resolve by source order, so inserting a new tl.set(..., 0) before existing tl calls would silently change which wins.
It can't happen here. The clamp only engages when the last located call precedes the timeline declaration — and located is in AST walk order, so "the last one is before the declaration" means every one is. There are no existing tl calls after the declaration to be reordered against. When any located call does follow the declaration, Math.max / the start comparison both leave the anchor exactly where it was.
The two writers get there differently and both are right on the chained case I was most suspicious of, const tl = gsap.timeline().to(...), where the located call is inside the declaration statement:
- acorn:
findEnclosingExpressionStatementfinds none (a declaration isn't an expression statement), solastCallEndis the end of the inner.to(...), which is less thantlDecl.end—Math.maxlifts it past the whole declaration. Correct. - recast: the statement path is the declaration, so
lastPath.node.start < timeline.node.startis false under the strict<, and the anchor stays on it. Correct.
A non-strict <= in the recast version would have been wrong here, which is worth noting because it reads like an arbitrary choice.
One nit
findInsertionPoint's docstring was rewritten in this diff, but only its first line. The second sentence still says "Returns null when no timeline declaration exists in the script — callers must not emit tl.xxx() calls in that case", and that's only true on the no-located-calls branch: with located calls and no declaration, it returns lastCallEnd (via tlDecl?.end ?? 0), not null. Pre-existing behaviour, unchanged by this PR — but the docstring was edited here, so it's the natural moment to make the second half match, or to say "null only when there are no existing calls and no declaration."
Status
gsapWriter.timelineOrder.test.ts 8/8 at this head; both mutations restored clean.
I did not run studio-server/src/routes/files.test.ts — that package's suite needs a workspace build and several files won't load in my environment (unrelated to this diff; it bit me on two other PRs tonight). The 28 added lines there rest on your runs.
Code merit only — no merge or queue action from me.
What
After dragging an element on the canvas and then resizing it, Studio saved the resize's size (a
tl.set) above the line that declares the timeline:tl.setruns beforeconst tlis initialised, so the saved film threwReferenceError: Cannot access 'tl' before initializationon every load and render. A project with no script yet ended up with the same order.The drag writes a global
gsap.set, which is placed above the timeline declaration. Both GSAP script writers inserted new timeline calls "after the last existing call", and after a drag that call is the globalgsap.set. They now insert after whichever comes later: the last existing call or the timeline declaration. The timeline variable name is read from the script, soconst master = gsap.timeline()works the same way.Where
Each writer has one function that picks where a new timeline call goes, and every insert goes through it:
findInsertionPointin the acorn writer (the default) andinsertAfterAnchorin the recast writer. Globalgsap.setplacement and the other paths already anchored on the declaration and are unchanged.Verification
gsapWriter.timelineOrder.test.ts: drag then resize, and drag then a keyframed tween, on both writers and withtlandmasteras the timeline name. Six cases fail on main and pass here. Resize alone and drag alone keep their exact output.files.test.ts: the script-less case through the real/gsap-mutationsroute fails on main and passes here.Not in this change: when a timeline call sits inside a wrapper function (for example an IIFE that declares its own timeline), the default writer still inserts after the wrapper, as on main.
Before
The saved film reloaded after drag, then resize: the
tlerror, no timeline registered, the card still at its original size.before-drag-resize-reload.webm
After
Same steps: no errors, the timeline is registered, and the card is at its resized size.
after-drag-resize-reload.webm