fix(parsers): emit global gsap.set position holds before the timeline declaration#2099
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
… declaration A base `gsap.set(...)` written AFTER the tween calls is wiped on the next soft reload: when a `from()` tween on the same target lazily initializes during a backwards render (the studio rebind's progress(0.0001) kick), GSAP reverts its internal isFromStart set, which removes the whole inline `transform` — taking the base set's x/y with it. The from() tween then re-parses the computed transform as identity and bakes x/y = 0 into the GSAP cache, so every element previously moved in the studio snaps back to its authored position whenever any other element is edited. Emitting the global set BEFORE the timeline construction makes it part of the pre-tween state the from() records, so every revert restores the moved pose instead of stripping it. - addAnimationToScript: global sets insert above the timeline declaration; the new-id lookup now diffs content-based ids instead of assuming the appended statement is last in source order. - updateAnimationInScript: a legacy trailing global set is relocated above the declaration whenever it's touched, healing files written before this change on the next nudge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1f04f4e to
e0b5d01
Compare
miga-heygen
left a comment
There was a problem hiding this comment.
LGTM with nits
Clean fix for a subtle GSAP ordering bug. The root cause analysis is spot-on — from() lazy-init during a backwards render (progress(0.0001, true) on rebind) reverts the inline transform, wiping any gsap.set() that was emitted after the tween calls. Hoisting the set above the timeline declaration makes it part of the pre-tween state that from() records and restores on revert.
What I liked:
- The fix is minimal and surgical — two insertion-point changes, one legacy relocation heal, no structural rewrites.
- The new-id detection in
addAnimationToScriptcorrectly diffs id-counts rather than assuming positional order. Handles duplicate ids gracefully. - The
updateAnimationInScriptrelocation is a nice "heal on touch" pattern — legacy files self-correct on the next nudge without a migration. - Test coverage is solid: insert-before, legacy relocation, already-hoisted no-op, and round-trip id resolution.
Minor nit (non-blocking):
The whitespace-collapsing ternary in the relocation logic is a bit dense:
const moveStart = /^\s*$/.test(script.slice(lineStart, exprStmt.start))
? lineStart
: exprStmt.start;A brief inline comment like // include leading blank prefix on the line would help future readers parse the intent faster.
Ship it.
— Miga
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at e0b5d01.
Nice piece of GSAP archaeology — the root-cause writeup matches what _initTween / _parseTransform actually do on the backwards-revert path (isFromStart nukes the whole inline transform, and any trailing gsap.set(x, y) on the same target is collateral). Hoisting the base set above the timeline declaration puts x/y into the pre-tween state from() records at init, so every revert restores it. Clean, minimal, well-tested.
Blockers
None.
Concerns
None structural. Two thin edge-case notes below — neither should gate.
Nits
- Legacy-heal
moveEndedge case inupdateAnimationInScript(gsapWriterAcorn.ts:399-404):moveEndextends pastexprStmt.endonly whenscript[exprStmt.end] === "\n". If a legacy file's trailinggsap.set(...)is the very last statement with no trailing newline, the move splices it into the same line as the timeline declaration (gsap.set(...);var tl = gsap.timeline(...);). Real HF studio scripts always landwindow.__timelines[...] = tl;after the set (which enforces a\n), so this doesn't fire in practice — but always appending a\nwhen the move doesn't consume one closes the hole cheaply. - Incremental healing is intentional and worth a body note: only the trailing set whose id matches
animationIdis relocated; other trailing sets in the same legacy file stay put until each is individually nudged. Worth a one-liner in the PR body so a future reader who inspects a partially-healed file doesn't file it as a bug.
Questions
- Multi-timeline scripts —
findTimelineDeclarationStatementusesparsed.timelineVar(scalar). If a script ever declares two named timelines, does the parser track both or just the first? Not blocking (every HF studio script I've seen is single-timeline), but if the parser holds a single value, the hoist targets that decl and any set correctly lands above it — noting the assumption for the record.
What I didn't verify
- The browser E2E claim (soft-reload cycles,
translate(267px, 20px)persistence) — trusted the writeup + the standalone-12-line repro claim. - Non-parser call sites of the writer API — public signatures haven't shifted (
{script, id}return, sameupdateAnimationInScriptargs), so no downstream should need changes. - Full CI at the new HEAD SHA — some jobs still
IN_PROGRESSat review time (Test,Build,Typecheck,Studio: load smoke,CLI smoke,Render on windows-latest). Prior run at the earlier SHA on this PR was fully green; scope is small enough that these should follow.
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed at e0b5d01c6d2e4e71e70c1cf46babe4a952a5e69c.
Additive to Miga and Rames: no new blocking findings from my pass.
What I verified:
packages/parsers/src/gsapWriterAcorn.ts:314computes the insertion point from the actual timeline declaration line start, andaddAnimationToScriptonly routes through it formethod === "set" && global, so normal timeline tween insertion stays on the existingfindInsertionPointpath.packages/parsers/src/gsapWriterAcorn.ts:397heals only touched legacy global sets by moving their expression statement above the declaration; that keeps the migration incremental and avoids a broader script rewrite.packages/parsers/src/gsapWriterAcorn.ts:516switches new-id detection to an id-count diff, which is the right fix once the inserted set is no longer source-order-last.packages/parsers/src/gsapWriter.acorn.test.ts:228and:256pin the two important behaviors: new global sets hoist before the timeline and legacy trailing sets relocate on update while already-hoisted sets stay put.
Verification: bun run --filter @hyperframes/parsers test -- gsapWriter.acorn.test.ts passed locally, 29/29. GitHub had no failed check runs at review time; two Windows jobs were still pending, so this approval is code-level and does not override CI.
Review by Magi.
Verdict: APPROVE
Reasoning: The parser write-path change is tightly scoped to off-timeline global position holds, preserves the existing tween path, and has focused regression coverage for both new insertion and legacy healing.

Problem
In the studio, moving one element resets every previously-moved element back to its authored position.
Repro:
#image-clip, then#video-clip— both persist as basegsap.set("#el", { x, y })lines and render correctly.#title-text(any element whose edit triggers a script rewrite + soft reload).#image-clipand#video-clipsnap back to their authored positions — live and on every subsequent render — even though thegsap.setlines are still in the file.Root cause
The writer emits base position holds after the tween calls:
On a soft reload the studio rebind kicks
progress(0.0001, true)synchronously — before GSAP's lazy queue flushes — so thefrom()tween on the same target first-initializes during a backwards render. GSAP's_initTweenthen reverts its internal isFromStart set, which removes the element's whole inlinetransform— taking the base set's x/y with it._parseTransformre-reads the computed transform as identity and bakesx/y = 0into the GSAP cache, so every later render writestranslate(0px, 0px).Standalone 12-line repro confirms: a base set emitted after a conflicting
from()tween loses its x/y under any seek once from-init runs; emitted before the timeline construction it survives every render order (sync seek, rewind kick, lazy flush, double from-tweens), because the from() records the set pose as its pre-tween state and every revert restores it.Fix
packages/parsers/src/gsapWriterAcorn.ts:addAnimationToScript— a globalgsap.setnow inserts on the line above the timeline declaration (findGlobalSetInsertionPoint). The new-id lookup diffs content-based ids against the pre-insert set instead of assuming the appended statement is last in source order.updateAnimationInScript— a legacy trailing global set is relocated above the declaration whenever it's touched, healing files written before this change on the next nudge.Testing
translate(267px, 20px)/translate(60px, 200px); the old trailing-set shape reproduces thetranslate(0px, 0px)reset.🤖 Generated with Claude Code