fix(parsers): a keyframe edit no longer overrides an earlier tween at the start (#4935)

* fix(parsers): a keyframe edit no longer overrides an earlier tween at the start

Writing a keyframe into a tween that starts later added a t=0 hold for every position prop of
its first keyframe, so a from() that already moves x before it lost its start value. The hold
now skips props an earlier tween on the same target writes; both writers share one helper.

* fix(parsers): keep the hold for label-placed tweens and re-sync one-line scripts stably

The recast parser leaves a label start unresolved; an unknown start no longer counts as earlier,
so the hold stays. The acorn insert now ends the hold on its own line when the next statement
shares the timeline line, matching what removal takes, so a second sync changes no bytes.

* fix(parsers): the recast hold sync reads tween starts from the acorn parse

The recast parser cannot resolve label positions, so an earlier label-placed tween looked
unordered and kept a hold it should not. Both writers now decide holds on the same parse.

* revert(parsers): keep the recast hold sync on its own parse

Deciding recast holds on the acorn parse lost selectors only recast resolves (an element bound
through a helper). Recast is the migration fallback and keeps its old hold for a start it cannot
resolve; the earlier-label case is decided by the default acorn writer, which resolves labels.

* style(parsers): name the hold insert's line ending instead of commenting it

* style(parsers): fit the hold helper's doc in four lines

* test(studio): bank the from/to edits this fix makes accurate and the resizes #4920 fixed

packages/parsers/src/gsapConstants.ts

@@ -76,15 +76,17 @@ type PositionWrite = Pick<
   "propertyGroup" | "properties" | "fromProperties" | "keyframes"
 >;
 
+function writesProperty(animation: PositionWrite, property: string): boolean {
+  return (
+    property in animation.properties ||
+    (!!animation.fromProperties && property in animation.fromProperties) ||
+    !!animation.keyframes?.keyframes.some((k) => property in k.properties)
+  );
+}
+
 /** A position write that sets x or y. An xPercent/yPercent centring set never duplicates one. */
 export function isXYPositionWrite(a: PositionWrite): boolean {
-  const xy = (props?: Record<string, unknown>) => !!props && ("x" in props || "y" in props);
-  return (
-    a.propertyGroup === "position" &&
-    (xy(a.properties) ||
-      xy(a.fromProperties) ||
-      !!a.keyframes?.keyframes.some((k) => xy(k.properties)))
-  );
+  return a.propertyGroup === "position" && (writesProperty(a, "x") || writesProperty(a, "y"));
 }
 
 export function classifyPropertyGroup(prop: string): PropertyGroupName {
@@ -107,6 +109,46 @@ export function classifyTweenPropertyGroup(
   return undefined;
 }
 
+function knownStart(animation: GsapAnimation): number | undefined {
+  if (animation.resolvedStart !== undefined) return animation.resolvedStart;
+  return typeof animation.position === "number" ? animation.position : undefined;
+}
+
+/**
+ * What a Studio hold pins from t=0 before a later keyframed tween: its first keyframe's position props,
+ * minus those an earlier timeline tween on the target writes (a global `gsap.set` is a base value).
+ */
+export function positionHoldForAnimation(
+  animation: GsapAnimation,
+  animations: readonly GsapAnimation[],
+): Record<string, number> | null {
+  if (!animation.keyframes) return null;
+  const start = knownStart(animation) ?? 0;
+  if (!(start > 0.001)) return null;
+  const first = [...animation.keyframes.keyframes].sort(
+    (left, right) => left.percentage - right.percentage,
+  )[0];
+  if (!first) return null;
+  // A tween whose start the parser could not resolve (a label, say) is not known to come first.
+  const earlier = animations.filter((other) => {
+    const otherStart = knownStart(other);
+    return (
+      other !== animation &&
+      !other.global &&
+      other.targetSelector === animation.targetSelector &&
+      otherStart !== undefined &&
+      otherStart < start - 0.001
+    );
+  });
+  const position: Record<string, number> = {};
+  for (const [property, value] of Object.entries(first.properties)) {
+    if (classifyPropertyGroup(property) !== "position" || typeof value !== "number") continue;
+    if (earlier.some((other) => writesProperty(other, property))) continue;
+    position[property] = value;
+  }
+  return Object.keys(position).length > 0 ? position : null;
+}
+
 export const SUPPORTED_EASES = [
   "none",
   "power1.in",

packages/parsers/src/gsapParser.test.ts

@@ -29,6 +29,7 @@ import {
   scalePositionsInScript,
 } from "./gsapParser.js";
 import type { GsapAnimation } from "./gsapParser.js";
+import { syncPositionHoldsBeforeKeyframes as syncPositionHoldsBeforeKeyframesAcorn } from "./gsapWriterAcorn.js";
 import { classifyPropertyGroup, classifyTweenPropertyGroup } from "./gsapConstants.js";
 import type { Keyframe } from "./types.js";
 import {
@@ -1797,6 +1798,64 @@ describe("keyframe mutations", () => {
       expect(syncPositionHoldsBeforeKeyframes(opacity)).not.toContain("hf-hold");
     });
 
+    // The from() owns x from t=0 until the keyframed tween; only y is left to hold.
+    const afterEarlierFrom =
+      `const tl = gsap.timeline({ paused: true });\n` +
+      `tl.from("#t", { x: -60, duration: 2, ease: "none" }, 0);\n` +
+      `tl.to("#t", { keyframes: { "0%": { x: -50, y: -3.5 }, "100%": { x: 60, y: 30 } }, duration: 1 }, 2);`;
+    it.each([
+      ["recast", syncPositionHoldsBeforeKeyframes],
+      ["acorn", syncPositionHoldsBeforeKeyframesAcorn],
+    ])("%s: holds no property an earlier tween on the target already writes", (_, sync) => {
+      const hold = parseGsapScript(sync(afterEarlierFrom)).animations.find(
+        (a) => a.method === "set",
+      );
+      expect(hold!.properties).toEqual({ y: -3.5, data: "hf-hold" });
+    });
+
+    it.each([
+      ["recast", syncPositionHoldsBeforeKeyframes],
+      ["acorn", syncPositionHoldsBeforeKeyframesAcorn],
+    ])("%s: still holds over a global gsap.set base value", (_, sync) => {
+      const script =
+        `gsap.set("#t", { x: 40 });\n` +
+        `const tl = gsap.timeline({ paused: true });\n` +
+        `tl.to("#t", { keyframes: { "0%": { x: -50 }, "100%": { x: 60 } }, duration: 1 }, 2);`;
+      const hold = parseGsapScript(sync(script)).animations.find(
+        (a) => a.method === "set" && !a.global,
+      );
+      expect(hold!.properties).toEqual({ x: -50, data: "hf-hold" });
+    });
+
+    it.each([
+      ["recast", syncPositionHoldsBeforeKeyframes],
+      ["acorn", syncPositionHoldsBeforeKeyframesAcorn],
+    ])("%s: still holds when the other tween sits at a label after the keyframes", (_, sync) => {
+      const script =
+        `const tl = gsap.timeline({ paused: true });\n` +
+        `tl.addLabel("later", 5);\n` +
+        `tl.to("#t", { x: 20, duration: 1 }, "later");\n` +
+        `tl.to("#t", { keyframes: { "0%": { x: -50 }, "100%": { x: 60 } }, duration: 1 }, 2);`;
+      const hold = parseGsapScript(sync(script)).animations.find((a) => a.method === "set");
+      expect(hold!.properties).toEqual({ x: -50, data: "hf-hold" });
+    });
+
+    it("acorn: a re-sync of a one-line script is byte-stable", () => {
+      const once = syncPositionHoldsBeforeKeyframesAcorn(
+        `const tl = gsap.timeline({ paused: true });tl.to("#t", { keyframes: { "0%": { x: -50 }, "100%": { x: 60 } }, duration: 1 }, 2);`,
+      );
+      expect(syncPositionHoldsBeforeKeyframesAcorn(once)).toBe(once);
+    });
+
+    it("acorn: holds nothing an earlier label-placed tween already writes", () => {
+      const script =
+        `const tl = gsap.timeline({ paused: true });\n` +
+        `tl.addLabel("early", 0);\n` +
+        `tl.from("#t", { x: -60, duration: 1 }, "early+=0.5");\n` +
+        `tl.to("#t", { keyframes: { "0%": { x: -50 }, "100%": { x: 60 } }, duration: 1 }, 2);`;
+      expect(syncPositionHoldsBeforeKeyframesAcorn(script)).not.toContain("hf-hold");
+    });
+
     it("removes an orphaned hold when its tween is gone", () => {
       const withHold = syncPositionHoldsBeforeKeyframes(posTweenAt(1.2));
       const tweenId = parseGsapScript(withHold).animations.find((a) => a.keyframes)!.id;

packages/parsers/src/gsapParser.ts

@@ -53,6 +53,7 @@ import {
   classifyPropertyGroup,
   classifyTweenPropertyGroup,
   isXYPositionWrite,
+  positionHoldForAnimation,
 } from "./gsapConstants";
 import type { PropertyGroupName } from "./gsapConstants";
 import { clipTweenMatcher, hasExplicitTime } from "./clipTweens";
@@ -1897,16 +1898,8 @@ export function syncPositionHoldsBeforeKeyframes(script: string): string {
     return result;
   }
   for (const anim of reparsed.animations) {
-    if (!anim.keyframes) continue;
-    const start = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0);
-    if (!(start > 0.001)) continue;
-    const firstKf = [...anim.keyframes.keyframes].sort((a, b) => a.percentage - b.percentage)[0];
-    if (!firstKf) continue;
-    const posProps: Record<string, number | string> = {};
-    for (const [k, v] of Object.entries(firstKf.properties)) {
-      if (classifyPropertyGroup(k) === "position" && typeof v === "number") posProps[k] = v;
-    }
-    if (Object.keys(posProps).length === 0) continue;
+    const posProps = positionHoldForAnimation(anim, reparsed.animations);
+    if (!posProps) continue;
     result = insertInheritedStateSet(result, anim.targetSelector, 0, {
       ...posProps,
       data: STUDIO_HOLD_MARKER,

packages/parsers/src/gsapWriterAcorn.ts

@@ -24,7 +24,11 @@ import {
   type ParsedGsapAcornForWrite,
   type TweenCallInfo,
 } from "./gsapParserAcorn.js";
-import { classifyPropertyGroup, isXYPositionWrite } from "./gsapConstants.js";
+import {
+  classifyPropertyGroup,
+  isXYPositionWrite,
+  positionHoldForAnimation,
+} from "./gsapConstants.js";
 import type { PropertyGroupName } from "./gsapConstants.js";
 import {
   findObjectArrayKeyframeIndex,
@@ -2436,7 +2440,8 @@ function insertInheritedStateSetInScript(
   const tlDecl = findTimelineDeclarationStatement(parsed.ast, parsed.timelineVar);
   const firstLocated = parsed.located[0];
   if (tlDecl) {
-    ms.appendLeft(tlDecl.end, "\n" + code);
+    const ownLineEnd = script[tlDecl.end] === "\n" ? "" : "\n";
+    ms.appendLeft(tlDecl.end, "\n" + code + ownLineEnd);
   } else if (firstLocated) {
     const firstCall = firstLocated.call;
     const exprStmt = findEnclosingExpressionStatement(firstCall.ancestors);
@@ -2462,36 +2467,6 @@ function removeStudioHoldSets(script: string, parsed: ParsedGsapAcornForWrite):
   return ms.toString();
 }
 
-function animationStart(animation: GsapAnimation): number {
-  if (animation.resolvedStart !== undefined) return animation.resolvedStart;
-  return typeof animation.position === "number" ? animation.position : 0;
-}
-
-function positionProperties(
-  properties: Record<string, number | string>,
-): Record<string, number | string> {
-  const position: Record<string, number | string> = {};
-  for (const [property, value] of Object.entries(properties)) {
-    if (classifyPropertyGroup(property) === "position" && typeof value === "number") {
-      position[property] = value;
-    }
-  }
-  return position;
-}
-
-function positionHoldForAnimation(
-  animation: GsapAnimation,
-): Record<string, number | string> | null {
-  if (!animation.keyframes) return null;
-  if (!(animationStart(animation) > 0.001)) return null;
-  const first = [...animation.keyframes.keyframes].sort(
-    (left, right) => left.percentage - right.percentage,
-  )[0];
-  if (!first) return null;
-  const position = positionProperties(first.properties);
-  return Object.keys(position).length > 0 ? position : null;
-}
-
 /** Acorn-native, byte-preserving hold synchronization used after mutations. */
 export function syncPositionHoldsBeforeKeyframes(script: string): string {
   const parsed = parseGsapScriptAcornForWrite(script);
@@ -2499,9 +2474,9 @@ export function syncPositionHoldsBeforeKeyframes(script: string): string {
   let result = removeStudioHoldSets(script, parsed);
   const current = parseGsapScriptAcornForWrite(result);
   if (!current) return result;
-  for (const entry of current.located) {
-    const animation = entry.animation;
-    const position = positionHoldForAnimation(animation);
+  const animations = current.located.map((entry) => entry.animation);
+  for (const animation of animations) {
+    const position = positionHoldForAnimation(animation, animations);
     if (!position) continue;
     result = insertInheritedStateSetInScript(result, animation.targetSelector, 0, {
       ...position,