Skip to content

fix(studio): a resize after a drag saves its size below the timeline it uses - #4638

Merged
miguel-heygen merged 1 commit into
mainfrom
fix/studio-timeline-writes-after-declaration
Sep 28, 2026
Merged

miguel-heygen merged 1 commit into
mainfrom
fix/studio-timeline-writes-after-declaration

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

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:

window.__timelines = window.__timelines || {};
gsap.set("#card", { x: -216, y: -40 });
tl.set("#card", { width: 751, height: 871 }, 0);
const tl = gsap.timeline({ paused: true });
window.__timelines["main"] = tl;

tl.set runs before const tl is initialised, so the saved film threw ReferenceError: Cannot access 'tl' before initialization on 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 global gsap.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, so const master = gsap.timeline() works the same way.

window.__timelines = window.__timelines || {};
gsap.set("#card", { x: -216, y: -40 });
const tl = gsap.timeline({ paused: true });
tl.set("#card", { width: 751, height: 871 }, 0);
window.__timelines["main"] = tl;

Where

Each writer has one function that picks where a new timeline call goes, and every insert goes through it: findInsertionPoint in the acorn writer (the default) and insertAfterAnchor in the recast writer. Global gsap.set placement 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 with tl and master as 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-mutations route fails on main and passes here.
  • Each rule turns a test red when broken.
  • The parsers suite and the studio-server route tests pass. Typecheck, lint, format and the audit gate pass.
  • In Studio, dragging a card and then resizing it on a fixture project, the saved file on main throws on reload. On this branch it reloads with no errors and the card at its new size.

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 tl error, no timeline registered, the card still at its original size.

Before: saved film throws "Cannot access 'tl' before initialization" and the card keeps 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: saved film loads with no errors and the card at its resized size

after-drag-resize-reload.webm

…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 somanshreddy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: findEnclosingExpressionStatement finds none (a declaration isn't an expression statement), so lastCallEnd is the end of the inner .to(...), which is less than tlDecl.end — Math.max lifts it past the whole declaration. Correct.
  • recast: the statement path is the declaration, so lastPath.node.start < timeline.node.start is 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.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Sep 28, 2026
Merged via the queue into main with commit e7a0b6b Sep 28, 2026
100 checks passed
@miguel-heygen
miguel-heygen deleted the fix/studio-timeline-writes-after-declaration branch September 28, 2026 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants