fix(parsers): unroll keeps tween order and label positions the parser read (#4157)

* test(parsers): pin unroll order and side-effect soundness

* fix(parsers): resolve helper tweens in source order and unroll only pure tween helpers

* fix(parsers): unroll keeps a helper whose tween chain ends in a non-tween call

A helper like tl.to(...).call(fn) was treated as adding only tweens, so unroll
dropped the callback. Only chains of set/to/from/fromTo count now.
Also reuses isFunctionNode and names the unroll scope.

* fix(parsers): keep chain order, label positions and chain callbacks when unrolling

Review found three ways unroll still changed a timeline:
- a helper call positioned by a label was read at 0 (label order used raw loc)
- a tween chain added by a helper ran in reverse (outer call stamped first)
- an inner chain link with a callback was unrolled and lost the callback

* fix(parsers): resolve a label defined inside an inlined helper at its call site

addLabel was never tagged during helper expansion, so a cloned label kept
its declaration-body offset while its sibling tweens were remapped to the
call site. Comparing the two put every such label before every tween.
addLabel now gets the same site/order stamp, with order as the tie-break
when a label and its own tween share a call site.

* test(parsers): assert the sibling-label unroll rewrite matches its parsed start

The only literal-rewrite path this fix actually exercises (the label-defining
helper itself is never unrolled, since onlyAddsTweens rejects addLabel).

* refactor(parsers): unroll's effect scan reuses acorn-walk like its sibling

containsEffect hand-rolled a generic object walk; hasUnsafeLoopIndexUse in
gsapWriterAcorn.ts already solves the same shape with acorn-walk.full.

packages/parsers/src/gsapInline.ts

@@ -20,7 +20,16 @@ import type { GsapProvenance } from "./gsapSerialize.js";
 type Node = any;
 
 /** Node keys that are metadata, not child AST to traverse/substitute. */
-const SKIP_KEYS = new Set(["type", "start", "end", "loc", "range", "__hfProvenance", "__hfOrder"]);
+const SKIP_KEYS = new Set([
+  "type",
+  "start",
+  "end",
+  "loc",
+  "range",
+  "__hfProvenance",
+  "__hfOrder",
+  "__hfSiteStart",
+]);
 
 const FUNCTION_TYPES = new Set([
   "ArrowFunctionExpression",
@@ -33,7 +42,7 @@ const GSAP_METHODS = new Set(["set", "to", "from", "fromTo"]);
 const MAX_DEPTH = 8;
 const MAX_ITERS = 512;
 
-function isFunctionNode(node: Node): boolean {
+export function isFunctionNode(node: Node): boolean {
   return !!node && FUNCTION_TYPES.has(node.type);
 }
 
@@ -161,6 +170,8 @@ interface ExpandCtx {
   site: { n: number };
   /** Mutable counter stamping expansion order onto tweens (clones share source loc). */
   order: { n: number };
+  /** Source offset of the top-level statement being expanded; tweens inherit it as their sort site. */
+  rootStart: number;
 }
 
 function walkNodes(node: Node, fn: (n: Node) => void): void {
@@ -181,13 +192,20 @@ function timelineRootName(call: Node): string | null {
   return obj?.type === "Identifier" ? obj.name : null;
 }
 
-function isTimelineRooted(call: Node, timelineVar: string): boolean {
+export function isTimelineRooted(call: Node, timelineVar: string): boolean {
   if (timelineRootName(call) !== timelineVar) return false;
   return (
     call.callee?.property?.type === "Identifier" && GSAP_METHODS.has(call.callee.property.name)
   );
 }
 
+/** True for `tl.addLabel(...)`: not a tween, but its position must sort in the same coordinate
+ *  space as tween calls, or a label defined inside an inlined helper resolves at its declaration
+ *  offset instead of its call site. */
+function isAddLabelCall(call: Node, timelineVar: string): boolean {
+  return timelineRootName(call) === timelineVar && call.callee?.property?.name === "addLabel";
+}
+
 function containsTimelineCall(node: Node, timelineVar: string): boolean {
   let found = false;
   walkNodes(node, (n) => {
@@ -463,12 +481,21 @@ function bodyStatements(node: Node): Node[] {
 /** Tag this body's direct timeline tweens with provenance + a monotonic expansion-order stamp. */
 function tagTimelineCalls(stmts: Node[], prov: GsapProvenance, ctx: ExpandCtx): void {
   for (const stmt of stmts) {
+    const calls: Node[] = [];
     walkNodes(stmt, (n) => {
-      if (n.type === "CallExpression" && isTimelineRooted(n, ctx.timelineVar)) {
-        tagProvenance(n, { ...prov });
-        n.__hfOrder = ctx.order.n++;
-      }
+      if (
+        n.type === "CallExpression" &&
+        (isTimelineRooted(n, ctx.timelineVar) || isAddLabelCall(n, ctx.timelineVar))
+      )
+        calls.push(n);
     });
+    // A chain's outer call is visited first but runs last; stamp in source order.
+    calls.sort((a, b) => a.callee.property.start - b.callee.property.start);
+    for (const n of calls) {
+      tagProvenance(n, { ...prov });
+      n.__hfOrder = ctx.order.n++;
+      n.__hfSiteStart = ctx.rootStart;
+    }
   }
 }
 
@@ -724,7 +751,11 @@ export function inlineComputedTimelines(
     depth: 0,
     site: { n: 0 },
     order: { n: 0 },
+    rootStart: 0,
   };
   const body = (ast.body ?? []).filter((stmt: Node) => !isHelperDecl(stmt, helpers));
-  ast.body = expandStatements(body, ctx);
+  ast.body = body.flatMap((stmt: Node) => {
+    ctx.rootStart = stmt.start;
+    return expandStatements([stmt], ctx);
+  });
 }

packages/parsers/src/gsapParserAcorn.ts

@@ -1749,7 +1749,23 @@ function collectAddLabelDefs(
   scope: ScopeBindings,
   sortedCalls: TweenCallInfo[],
 ): AddLabelDef[] {
-  const callLocs = sortedCalls.map((c) => c.node.callee?.property?.loc?.start);
+  // Clones from the same expansion share __hfSiteStart (see tagTimelineCalls), so a
+  // label and a sibling tween can tie on site; __hfOrder is the tie-break, matching
+  // the source order tagTimelineCalls stamped them in.
+  const callKeys = sortedCalls.map((c) => ({
+    site: callSiteStart(c),
+    order: c.node.__hfOrder as number | undefined,
+  }));
+  const comesAfter = (
+    site: number | undefined,
+    order: number | undefined,
+    labelSite: number,
+    labelOrder: number | undefined,
+  ): boolean => {
+    if (site === undefined) return false;
+    if (site !== labelSite) return site > labelSite;
+    return (order ?? -Infinity) > (labelOrder ?? -Infinity);
+  };
   const defs: AddLabelDef[] = [];
   acornWalk.simple(ast, {
     // fallow-ignore-next-line complexity
@@ -1772,14 +1788,13 @@ function collectAddLabelDefs(
       const posVal = resolveNode(node.arguments?.[1], scope);
       const position =
         typeof posVal === "number" || typeof posVal === "string" ? posVal : undefined;
-      const labelLoc = callee.property?.loc?.start;
+      // Same coordinate space as callSiteStart: a label cloned into an inlined
+      // helper keeps the declaration's own offset unless tagTimelineCalls stamped it.
+      const labelStart: number | undefined = node.__hfSiteStart ?? callee.property?.start;
+      const labelOrder: number | undefined = node.__hfOrder;
       let order = sortedCalls.length;
-      if (labelLoc) {
-        order = callLocs.findIndex(
-          (l) =>
-            l &&
-            (l.line > labelLoc.line || (l.line === labelLoc.line && l.column > labelLoc.column)),
-        );
+      if (labelStart !== undefined) {
+        order = callKeys.findIndex((k) => comesAfter(k.site, k.order, labelStart, labelOrder));
         if (order === -1) order = sortedCalls.length;
       }
       defs.push({ name, position, order });
@@ -1795,15 +1810,20 @@ function compareByLoc(a: TweenCallInfo, b: TweenCallInfo): number {
   return aLoc.line - bLoc.line || aLoc.column - bLoc.column;
 }
 
-// Inlined tweens carry a monotonic __hfOrder (clones share source loc, so loc
-// can't order them); they sort by that, after all literal (loc-ordered) tweens.
+// Clones of an inlined helper share source loc, so an inlined tween sorts at the top-level
+// statement that expanded it (__hfSiteStart), then by expansion order (__hfOrder).
+function callSiteStart(call: TweenCallInfo): number | undefined {
+  return call.node.__hfSiteStart ?? call.node.callee?.property?.start;
+}
+
 function compareCallOrder(a: TweenCallInfo, b: TweenCallInfo): number {
+  const aSite = callSiteStart(a);
+  const bSite = callSiteStart(b);
+  if (aSite !== undefined && bSite !== undefined && aSite !== bSite) return aSite - bSite;
   const ao = a.node.__hfOrder;
   const bo = b.node.__hfOrder;
-  if (ao === undefined && bo === undefined) return compareByLoc(a, b);
-  if (ao === undefined) return -1;
-  if (bo === undefined) return 1;
-  return ao - bo;
+  if (ao !== undefined && bo !== undefined) return ao - bo;
+  return compareByLoc(a, b);
 }
 
 function sortBySourcePosition(calls: TweenCallInfo[]): void {

packages/parsers/src/gsapUnroll.test.ts

@@ -155,4 +155,93 @@ fade("#b", 2);
 ${call};`;
     expect(unrollComputedTimeline(script)).not.toContain("function fade");
   });
+
+  describe("keeps the timeline the parser read", () => {
+    const starts = (script: string) =>
+      Object.fromEntries(
+        parseGsapScriptAcorn(script).animations.map((a) => [a.targetSelector, a.resolvedStart]),
+      );
+
+    const script = `const tl = gsap.timeline();
+function fade(s) { tl.to(s, { opacity: 0, duration: 1 }); }
+fade("#a"); tl.to("#c", { x: 1, duration: 1 }); fade("#b");`;
+
+    it("resolves helper tweens in source order among literal tweens", () => {
+      expect(starts(script)).toEqual({ "#a": 0, "#c": 1, "#b": 2 });
+    });
+
+    it("unrolls to a script whose tweens start at the same times", () => {
+      expect(starts(unrollComputedTimeline(script))).toEqual(starts(script));
+    });
+
+    it("places helper tweens at the label they name, and unrolls to the same starts", () => {
+      const labelled = `const tl = gsap.timeline();
+function fade(s, at) { tl.to(s, { opacity: 0, duration: 1 }, at); }
+tl.addLabel("mid", 2);
+fade("#a", "mid"); fade("#b", "mid+=0.5");
+tl.to("#c", { x: 1, duration: 1 }, "mid");`;
+      expect(starts(labelled)).toEqual({ "#a": 2, "#b": 2.5, "#c": 2 });
+      expect(starts(unrollComputedTimeline(labelled))).toEqual(starts(labelled));
+    });
+
+    it("keeps the order of a tween chain a helper adds", () => {
+      const chained = `const tl = gsap.timeline();
+function f() { tl.to("#a", { opacity: 0, duration: 1 }).to("#b", { x: 5, duration: 2 }).to("#c", { y: 7, duration: 3 }); }
+f();`;
+      expect(starts(chained)).toEqual({ "#a": 0, "#b": 1, "#c": 3 });
+      expect(starts(unrollComputedTimeline(chained))).toEqual(starts(chained));
+    });
+
+    it("resolves a tween positioned by a label defined inside a helper body", () => {
+      const labelInHelper = `const tl = gsap.timeline();
+function group(s) { tl.addLabel("mid"); tl.to(s, { opacity: 0, duration: 1 }); }
+tl.to("#pre", { x: 1, duration: 1 });
+group("#a");
+tl.to("#post", { x: 2, duration: 1 }, "mid");`;
+      expect(starts(labelInHelper)).toEqual({ "#pre": 0, "#a": 1, "#post": 1 });
+    });
+
+    it("resolves a helper that only reads a label defined inside a sibling helper", () => {
+      const siblingLabel = `const tl = gsap.timeline();
+function makeLabel(s) { tl.addLabel("mid"); tl.to(s, { opacity: 0, duration: 1 }); }
+function useLabel(s) { tl.to(s, { x: 1, duration: 1 }, "mid"); }
+tl.to("#pre", { y: 1, duration: 1 });
+makeLabel("#a");
+useLabel("#b");`;
+      expect(starts(siblingLabel)).toEqual({ "#pre": 0, "#a": 1, "#b": 1 });
+      // useLabel only adds a tween, so it unrolls to a literal at the label's resolved start.
+      expect(starts(unrollComputedTimeline(siblingLabel))).toEqual(starts(siblingLabel));
+    });
+  });
+
+  it("leaves a helper as authored when an inner link of its tween chain has a callback", () => {
+    const script = `const tl = gsap.timeline();
+function f(s) { tl.to(s, { opacity: 0, duration: 1, onComplete: () => { window.hit++; } }).to(s, { x: 1, duration: 1 }); }
+f("#a");`;
+    expect(unrollComputedTimeline(script)).toBe(script);
+  });
+
+  it("leaves a helper call as authored when the helper does more than add tweens", () => {
+    const script = `const tl = gsap.timeline();
+function fade(sel, at) { window.count++; tl.to(sel, { opacity: 1, duration: 1 }, at); }
+fade("#a", 1);`;
+    expect(unrollComputedTimeline(script)).toBe(script);
+  });
+
+  it("leaves a loop as authored when its body sets state outside the timeline", () => {
+    const script = `const tl = gsap.timeline();
+for (let i = 0; i < 2; i++) { gsap.set("#x", { opacity: 0 }); tl.to("#x", { opacity: 1, duration: 1 }, i); }`;
+    expect(unrollComputedTimeline(script)).toBe(script);
+  });
+
+  it.each([
+    ["a tl.call callback", "tl.call(() => window.hit++, [], at);"],
+    ["a label", 'tl.addLabel("mark", at);'],
+    ["a call chained after a tween", "tl.to(sel, { opacity: 1, duration: 1 }, at).call(() => {});"],
+  ])("leaves a helper as authored when it adds %s", (_case, body) => {
+    const script = `const tl = gsap.timeline();
+function fade(sel, at) { ${body} }
+fade("#a", 1);`;
+    expect(unrollComputedTimeline(script)).toBe(script);
+  });
 });

packages/parsers/src/gsapUnroll.ts

@@ -15,10 +15,12 @@
  * inside other helpers) are left as-is rather than guessed at.
  */
 import * as acorn from "acorn";
+import * as acornWalk from "acorn-walk";
 import MagicString from "magic-string";
 import type { GsapAnimation } from "./gsapSerialize.js";
 import { serializeValue as valueToCode, safeJsKey as safeKey } from "./gsapSerialize.js";
 import { parseGsapScriptAcorn } from "./gsapParserAcorn.js";
+import { isFunctionNode, isTimelineRooted } from "./gsapInline.js";
 
 // acorn nodes are structurally untyped here.
 type Node = any;
@@ -100,9 +102,7 @@ function declaresFunction(stmt: Node): boolean {
   if (stmt.type === "FunctionDeclaration") return true;
   return (
     stmt.type === "VariableDeclaration" &&
-    (stmt.declarations ?? []).some((d: Node) =>
-      ["FunctionExpression", "ArrowFunctionExpression"].includes(d.init?.type),
-    )
+    (stmt.declarations ?? []).some((d: Node) => isFunctionNode(d.init))
   );
 }
 
@@ -146,16 +146,98 @@ function unreferencedHelperDecls(
   return dead;
 }
 
-/** A literal tween cannot encode an unknown start, duration or selector, so those statements stay as authored. */
-function dropStatementsWithUnknownTiming(byStatement: Map<Node, GsapAnimation[]>): void {
-  for (const [stmt, anims] of byStatement) {
-    if (
-      anims.some(
-        (a) => a.durationUnresolved || a.resolvedStart === undefined || a.hasUnresolvedSelector,
-      )
-    ) {
-      byStatement.delete(stmt);
+const EFFECT_NODES = new Set([
+  "CallExpression",
+  "NewExpression",
+  "UpdateExpression",
+  "AssignmentExpression",
+]);
+
+function containsEffect(node: Node): boolean {
+  let hit = false;
+  acornWalk.full(node, (n: Node) => {
+    if (EFFECT_NODES.has(n.type)) hit = true;
+  });
+  return hit;
+}
+
+/** True for a chain like `tl.to(...).from(...)` where every link is a tween method with effect-free arguments. */
+function isPureTweenChain(expr: Node, timelineVar: string): boolean {
+  let node = expr;
+  while (node?.type === "CallExpression") {
+    if (!isTimelineRooted(node, timelineVar) || node.arguments.some(containsEffect)) return false;
+    node = node.callee?.object;
+  }
+  return node?.type === "Identifier";
+}
+
+const blockStatements = (block: Node): Node[] =>
+  block?.type === "BlockStatement" ? block.body : block ? [block] : [];
+
+/** Statements a `forEach` callback runs, or null when the call is not a `forEach` with a function. */
+function forEachBody(call: Node): Node[] | null {
+  const callback = call.callee?.property?.name === "forEach" ? call.arguments[0] : null;
+  return callback?.body ? blockStatements(callback.body) : null;
+}
+
+/** Statements a helper call or a `forEach` callback runs, or null when the call is neither. */
+function calledBody(call: Node, helpers: Map<string, Node[]>): Node[] | null {
+  if (call?.type !== "CallExpression") return null;
+  if (call.callee?.type !== "Identifier") return forEachBody(call);
+  return helpers.get(call.callee.name) ?? null;
+}
+
+/** Statements of the helper a call statement invokes, or of the loop it is, else null. */
+function bodyOf(stmt: Node, helpers: Map<string, Node[]>): Node[] | null {
+  if (stmt.type === "ExpressionStatement") return calledBody(stmt.expression, helpers);
+  return stmt.body ? blockStatements(stmt.body) : null;
+}
+
+interface UnrollScope {
+  timelineVar: string;
+  helpers: Map<string, Node[]>;
+}
+
+/** True when running `stmts` only adds tweens to the timeline, so replacing them with literals drops nothing. */
+function onlyAddsTweens(stmts: Node[], ctx: UnrollScope, seen: Set<Node[]> = new Set()): boolean {
+  if (seen.has(stmts)) return false;
+  seen.add(stmts);
+  return stmts.every((stmt) => {
+    if (stmt.type === "VariableDeclaration") return !containsEffect(stmt);
+    if (stmt.type === "ExpressionStatement" && isPureTweenChain(stmt.expression, ctx.timelineVar)) {
+      return true;
     }
+    const nested = bodyOf(stmt, ctx.helpers);
+    return nested !== null && onlyAddsTweens(nested, ctx, seen);
+  });
+}
+
+/** `[name, body]` for each function a top-level statement declares. */
+function declaredFunctions(stmt: Node): Array<[string, Node[]]> {
+  if (stmt.type === "FunctionDeclaration") {
+    return stmt.id?.name ? [[stmt.id.name, blockStatements(stmt.body)]] : [];
+  }
+  if (stmt.type !== "VariableDeclaration") return [];
+  return stmt.declarations
+    .filter((d: Node) => d.id?.name && isFunctionNode(d.init))
+    .map((d: Node): [string, Node[]] => [d.id.name, blockStatements(d.init.body)]);
+}
+
+/** Function declarations and function-valued consts by name, with their body statements. */
+const collectHelperBodies = (statements: Node[]): Map<string, Node[]> =>
+  new Map(statements.flatMap(declaredFunctions));
+
+/** Statements stay as authored when literal tweens cannot encode their timing, or they do more than add tweens. */
+function dropStatementsUnsafeToUnroll(
+  byStatement: Map<Node, GsapAnimation[]>,
+  ctx: UnrollScope,
+): void {
+  for (const [stmt, anims] of byStatement) {
+    const unknownTiming = anims.some(
+      (a) => a.durationUnresolved || a.resolvedStart === undefined || a.hasUnresolvedSelector,
+    );
+    const bodyStmts = unknownTiming ? null : bodyOf(stmt, ctx.helpers);
+    if (bodyStmts === null || !onlyAddsTweens(bodyStmts, ctx)) byStatement.delete(stmt);
   }
 }
 
@@ -189,7 +271,10 @@ export function unrollComputedTimeline(script: string): string {
   const grouped = groupByTopLevelStatement(computed, statements);
   if (!grouped) return script;
   const { byStatement, helperNames } = grouped;
-  dropStatementsWithUnknownTiming(byStatement);
+  dropStatementsUnsafeToUnroll(byStatement, {
+    timelineVar: parsed.timelineVar,
+    helpers: collectHelperBodies(statements),
+  });
   if (byStatement.size === 0) return script;
 
   const ms = new MagicString(script);