Skip to content

Commit 8c981a4

Browse files
vanceingallsclaude
andauthored
fix(studio): restore timeline move/resize fallback parity (review #1466) (#1539)
* fix(studio): restore timeline move/resize fallback parity (review #1466) The §3.2 sdkTimingPersist rewrite regressed the non-SDK fallback path vs the pre-cutover behavior. Restored, on both fallback entry points (no-session and sdkTimingPersist-returned-unhandled): - Resize live DOM patch dropped the conditional data-playback-start/media-start attr — restored so a start-trim updates the preview's in-point immediately. - Move/resize fallback dropped the GSAP-position sync (shift/scaleGsapPositions) + reloadPreview — restored so server-path edits keep GSAP tweens in sync and refresh the preview (the SDK path folds both into setTiming). - Undo-coalesce drift: fallback enqueueEdit carried no coalesceKey while the SDK branch did — plumbed coalesceKey through persistTimelineEdit so undo granularity is identical on either path. - Documented the hasPbsAdjustment second clause + sdkTimingPersist before-capture transition limitation. Flag-off (dark launch) so this lands as one fix PR at the stack tip rather than restacking the mid-stack §3.2 commit. #1500 review items: parity-harness gap already closed at the tip (arc/unroll recast-vs-acorn parity added); blockRemoveRange flagged 'potential' but verified correct (no comma residue on any block position). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk): retire duplicate removeGsapKeyframe keyframeIndex variant (review #1498) EditOp had two removeGsapKeyframe members with the same discriminant but different shapes (keyframeIndex vs percentage) — TS can't discriminate them and a handler could get the wrong shape. Per both reviewers (option 2): retire the keyframeIndex variant. It had no production caller (Studio dispatches percentage only); removed the dead by-index handleRemoveGsapKeyframe + simplified the dispatcher. resolveKeyframe stays (setGsapKeyframe still uses keyframeIndex). Converted the one by-index test to the percentage API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): gate ALL cutover persist paths on the flag — true dark launch (review #1469 finding #6) Only sdkCutoverPersist (style/text/attr) checked STUDIO_SDK_CUTOVER_ENABLED. sdkTimingPersist, dispatchGsapOpAndPersist (every GSAP op) and sdkDeletePersist guarded only on `!sdkSession` — and useSdkSession opens a session by default for shadow/selection, so timing/GSAP/keyframe/delete cutover was ALWAYS live regardless of the flag. Flipping the flag OFF could not disable it, so the data-loss bugs in those paths (single-prop wipe, wrong-keyframe match, tween collapse, arc strip) ship LIVE on merge instead of being dark-launched. Added the flag guard at all three chokepoints → flag OFF returns false → callers fall back to the legacy server path. Makes the stack genuinely dark-launchable: merge is now a no-op in prod, and the remaining cutover correctness bugs become flip-prerequisites rather than merge-blockers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core,sdk): correct 8 GSAP write-path review findings (#1539) Eight correctness bugs from the SDK-cutover review. Several were cases where BOTH writers were identically wrong, so the recast-vs-acorn parity suite stayed green; the new tests assert the real-world-correct result, not agreement. - #2 findKfPropByPct: match the CLOSEST keyframe within tolerance, not the first within 2% — removing/updating 50% on 0/49/50/100 no longer hits 49%. - #3 handleSetTiming: shift each tween by the start DELTA and scale duration by the clip-duration RATIO per-tween, instead of writing absolute newStart/ newDuration onto every tween (which collapsed staggers and blew durations). - #4 enableArcPath: insert motionPath via appendRight at the object start so the insertion can't collide with the x/y remove-range end (which made MagicString discard the append and emit '{}'). - #5 splitAnimationsInScript: compute the inherited baseline in a forward pre-pass so the split-spanning midpoint sees earlier tweens (the reverse write loop is kept for stable count-suffixed ids). - #9 unrollDynamicAnimations: preserve non-target loop-body statements (e.g. tl.set initial-state) per iteration instead of overwriting the whole loop. - #10 buildMotionPathObjectCode (both writers): emit the cubic form when segment curviness varies so per-segment curviness survives, not just segments[0]. - #11 readLastWaypointXY: handle UnaryExpression so negative destination coords are recovered when disabling an arc path. - #15 no-bang: removed every `!` non-null assertion in the touched files, replaced with guards/fallbacks. Tests: gsapWriter.reviewFixes.test.ts (#2/#4/#5/#9/#10/#11) and mutate.gsap.test.ts setTiming GSAP-sync block (#3). All fail on the base and pass after the fix; tsc + full core/sdk suites + parity stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): SDK cutover review fixes — merge tween props, stabilize debounce, serialize gsap writes, on-disk undo baseline, self-write identity Addresses 5 SDK-cutover review findings (studio-only): - #1 useGsapPropertyDebounce: editing one GSAP tween property no longer drops the tween's other animated props. setGsapTween REPLACES the property set, so merge the single edit into the tween's CURRENT properties (read from the SDK doc) before dispatching, mirroring the legacy server merge. - #7 useGsapPropertyDebounce: stabilize the flush callback by reading sdk deps from a ref instead of an unmemoized literal, so a parent re-render mid-edit no longer tears down + flushes the debounce (one commit/undo entry per render). - #8 sdkCutover/useGsapScriptCommits: route SDK gsap-write persists through the same per-file keyed serializer the legacy commitMutation uses, so concurrent same-file read-modify-writes can't interleave and lose an edit. - #12 sdkCutover/useTimelineEditing: capture the exact on-disk bytes as the undo 'before' for timing/GSAP persists (matching the style/delete paths) instead of a normalized SDK serialize() re-emit that reformatted the whole file on undo. - #14 useSdkSession/sdkSelfWriteRegistry: discriminate a cutover echo from an undo write by CONTENT identity (registered self-write hash), not just the 2 s timestamp window — an undo write always reloads the SDK session. Tests: useGsapPropertyDebounce(.test), useGsapPropertyDebounceFlush.test, sdkSelfWriteRegistry.test, and new sdkCutover.test cases; each reproduces the review scenario and asserts the corrected behavior (verified red before fix). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(core): extract split/collapse helpers to satisfy no-fallow-ignore rule The #5 (split) and #15 (no-bang guards) fixes pushed splitAnimationsInScript and removeAllKeyframesFromScript over fallow's complexity threshold, and a fallow-ignore had been added to splitAnimationsInScript. Per the hard rule (never ignore — fix), extracted buildSpanningSplit + applyTweenSplit (split) and buildCollapsedFlatVars (collapse), and removed the ignore. Both functions now under threshold; fallow new-only gate reports 0 new findings. Behavior unchanged — core 1811 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(studio): pin dark-launch flag-gate contract (review #1539, Rames/Via) flag OFF ⇒ sdkTimingPersist / sdkGsapTweenPersist (GSAP-op chokepoint) / sdkDeletePersist all return false even with a valid session → legacy fallback. The prod flag-flip rests on this contract; sdkCutover.test.ts only mocks the flag TRUE, so a future gate refactor could silently re-enable cutover on flag-off without failing CI. This sibling file mocks it FALSE and locks the three guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(studio): leading flag-gate on sdkGsapTweenPersist (review #1539 nit, Via) The add-op getElement existence check ran before the inner gate, so flag-off did an SDK touch before falling back. Lead with the flag guard to match the other three chokepoints — flag-off is now a clean no-op at every entry point. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): unroll-preservation regressions — non-for loops + AST index substitution (review R2) The #9 unroll-preservation fix had two confirmed regressions: - Non-for loops (forEach/for-of/for-in/while): loopIndexVarName returns null, so substitution no-op'd and preserved siblings kept a now-undefined loop variable (e.g. `item`) → ReferenceError at render. Now returns null for those forms → caller falls back to the blanket loop overwrite (drops siblings, valid code). The #9 fixture only used `for(let i…)` so it never caught this. - substituteLoopIndex did a \bvar\b regex over raw source including string literals, corrupting selectors like ".row-i" → ".row-0". Now AST-based: substitutes only real Identifier uses, skipping string literals and non-computed member/key positions (extracted isIndexBindingPosition helper to stay under the fallow complexity threshold — no ignore added). Two regression tests added (forEach no-dangling-var; for-loop string-literal intact). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk,core): unrollDynamicAnimations rejects empty element list (R1 #1501b) An empty `elements` array has no unrolled form — the writer would overwrite the loop/statement with zero tween calls, silently deleting the animation. - gsapWriterAcorn: unrollDynamicAnimations returns the script verbatim on an empty list (no-op instead of a destructive overwrite). - validateOp: reject unrollDynamicAnimations with empty elements as E_INVALID_ARGS so callers get a clean error rather than silent corruption. - Tests: writer no-op on []; validateOp E_INVALID_ARGS on []. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(sdk): cache draft element in applyDraft, drop HTMLElement casts (R1 #1490a) applyDraft runs at 60fps during a drag but re-ran doc.querySelector on every call — the _draftEl/_draftId fields were only consumed by commit/cancel, never to skip the query. Reuse the tracked element when the id matches and the node is still connected; re-query only on id change or detach (iframe reload). Retypes _draftEl to HTMLElement | null (only ever set from querySelector<HTMLElement>), which removes the `as HTMLElement` casts in commitPreview / _clearDraft. Test asserts a repeated same-id drag queries once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk,core): round-3 correctness — unroll AST safety, single-dispatch undo, empty-arg guards, persist decouple Addresses the highest-severity round-3 review findings: - gsapWriterAcorn unroll (R3 #1/#2/#9): the round-2 AST-substitution fix emitted invalid GSAP for object shorthand `{ i }` (→ `{ 0 }`) and shadowed inner bindings (→ `for(let i=0;0<3;0++)`), and silently dropped sibling statements on non-`for` loops (forEach/for-of). The unroll now REFUSES (no-ops, leaving the dynamic loop intact) whenever siblings can't be safely reproduced — a non-`for` loop, an unmodeled statement, or an unsafe index use — instead of dropping or corrupting. Plain `for` loops with safe siblings still unroll. - session single-dispatch undo (R3 #5/#11): _dispatch now reverses the inverse patch list (parity with batch()). A single op emitting order-dependent inverse patches — a nested parent+child removeElement, an aliased multi-target — undid forward and dropped the child subtree / landed on an intermediate value. - materializeKeyframes empty-array (R3 #10): the unguarded twin of the just-fixed unrollDynamicAnimations. Writer no-ops on an empty keyframe list; validateOp rejects it as E_INVALID_ARGS (shared gsapScriptMissing helper). - history:false persist decouple (R3 #4): persist (auto-save) no longer lives inside the history-enable block, so opting out of SDK undo no longer silently disables all disk writes (data-loss trap for #1496's flag consumers). Tests: unroll refuse cases (shorthand/shadow/forEach) + safe-for-loop regression; nested removeElement undo; materializeKeyframes writer no-op + validateOp reject; history:false-still-persists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): stripGsapForId re-parses per removal so all tweens for a deleted element are stripped (R3 #3) Animation ids are count-based (positional), so removing one tween renumbers the survivors. stripGsapForId captured every matching id from a single up-front parse then removed against the mutating script — after the first removal the later ids were stale and silently no-op'd, leaving an orphaned tl.to() referencing the just-deleted element. Now re-parse after each removal and strip the first still-matching animation until none remain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(core): gsap writer — keyframe ease routing, convert preserves delay, addLabel dedup (R3 #7/#8/#12) - #7: updateAnimationInScript routes an ease update on a keyframe tween to keyframes.easeEach (per-keyframe), not a top-level ease that GSAP ignores — the user's keyframe-easing edit was silently a no-op. - #8: convertToKeyframesFromScript now preserves every non-editable vars key (delay/callbacks/stagger/yoyo/…) verbatim via preservedVarsEntries instead of rebuilding from the GsapAnimation object, which had no `delay` field and dropped it — shifting the tween's start time. - #12: addLabelToScript moves an existing same-named label (overwrites its position) instead of appending a duplicate; duplicates made removeLabel over-remove (it deletes every match, including a pre-existing label). Tests: easeEach routing, delay preservation, addLabel move-not-duplicate + hand-authored-dup removal. Updated the old "no dedup contract" corpus test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk): handleSetTiming #domId + data-duration sync; validateOp resolves ids + arc/selector (R3 #6/#13, CF2 #15/#16) CF2 #15: handleSetTiming re-synced GSAP tweens only when the selector matched the element's hf-id. The common #domId-targeted tween (authored by the Studio panel) never matched, so moving/resizing a clip via the SDK timing path left its animations unsynced. Now match the tween selector against the DOM id too. CF2 #16: handleSetTiming read/wrote only data-end. Clips authored with data-duration (what the runtime prefers) got a fresh data-end beside a stale data-duration (no playback change) and oldDuration=null collapsed the GSAP duration-scale ratio to 1. Now read duration preferring data-duration, and write back to whichever attribute the clip uses (timingPath gains a "duration" field). R3 #13b: deleteAllForSelector compared selectors with strict === and missed the alternate quote style ([data-hf-id='x'] vs "x"); now quote-insensitive. R3 #6/#13a: validateOp now resolves the animationId for id-bearing GSAP ops (E_TARGET_NOT_FOUND instead of a misleading ok that no-ops at apply), and updateArcSegment validates the arc is enabled + the segment index is in range. Tests: #domId move sync, data-duration resize + scale, quote-insensitive delete, unresolved-id rejection, arc-segment preconditions. Updated the loose-can() test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(core,sdk): name the acorn-node type alias; keyToPath round-trips timing.duration (R3 #14) - gsapWriterAcorn: replace the bare `: any` AST-node annotations with the named `type Node = any` alias, matching the established convention in gsapParserAcorn.ts / gsapInline.ts ("acorn ESTree nodes are structurally untyped"). Documents intent and is greppable; type-identical (zero runtime change). A full ESTree typing is a deliberate architecture decision the codebase has not taken and is out of scope here. - patches: keyToPath/timingPath now include the "duration" timing field added for the data-duration resize fix, so a timing.duration override round-trips on T3 replay instead of being dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sdk): cascadeRemoveAnimations re-parses per removal (R4 — SDK twin of #3) cascadeRemoveAnimations captured every matching animation id from a single up-front parse, then removed against the mutating script — the SDK-side twin of the stripGsapForId bug (R3 #3). Animation ids are positional, so removing the first tween for an element renumbered the survivors and the stale later ids no-op'd, orphaning those tweens on the just-removed element. Now re-parse after each removal and strip the first still-matching animation until none remain. Also adds the reviewer's defense-in-depth test: an aliased multi-target setStyle (same id twice) undoes to the original, not the intermediate (exercises the single-dispatch inverse reversal from R3 #5/#11). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 09cefc1 commit 8c981a4

29 files changed

Lines changed: 2319 additions & 343 deletions

packages/core/src/parsers/gsapParser.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2170,9 +2170,16 @@ function buildMotionPathObjectCode(config: {
21702170
}): string {
21712171
const { waypoints, segments, autoRotate } = config;
21722172
const hasExplicitControlPoints = segments.some((s) => s.cp1 && s.cp2);
2173+
// The simple `path` array supports only one scalar curviness for the whole
2174+
// path, so per-segment curviness must use the cubic form (curviness baked into
2175+
// each segment's control points). Without this, the simple branch serializes
2176+
// only segments[0].curviness and silently drops every other segment's curve.
2177+
const curvinessVaries = segments.some(
2178+
(s) => (s.curviness ?? 1) !== (segments[0]?.curviness ?? 1),
2179+
);
21732180

21742181
let pathEntries: string[];
2175-
if (hasExplicitControlPoints && waypoints.length >= 2) {
2182+
if ((hasExplicitControlPoints || curvinessVaries) && waypoints.length >= 2) {
21762183
// type: "cubic" — interleave control points: [anchor, cp1, cp2, anchor, ...]
21772184
pathEntries = [`{x: ${waypoints[0]!.x}, y: ${waypoints[0]!.y}}`];
21782185
for (let i = 0; i < segments.length; i++) {

packages/core/src/parsers/gsapSerialize.ts

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,20 @@ export function buildArcPath(
120120
autoRotate: boolean | number,
121121
isCubic: boolean,
122122
): MotionPathShape | undefined {
123-
if (coords.length < 2) return undefined;
123+
const first = coords[0];
124+
if (coords.length < 2 || !first) return undefined;
124125
const segments: ArcPathSegment[] = [];
125126
let waypoints: Array<{ x: number; y: number }>;
126127
if (isCubic && coords.length >= 4) {
127128
// coords are [anchor, cp1, cp2, anchor, cp1, cp2, anchor, ...].
128-
waypoints = [coords[0]!];
129+
waypoints = [first];
129130
for (let i = 1; i + 2 < coords.length; i += 3) {
130-
waypoints.push(coords[i + 2]!);
131-
segments.push({ curviness, cp1: coords[i]!, cp2: coords[i + 1]! });
131+
const cp1 = coords[i];
132+
const cp2 = coords[i + 1];
133+
const anchor = coords[i + 2];
134+
if (!cp1 || !cp2 || !anchor) continue;
135+
waypoints.push(anchor);
136+
segments.push({ curviness, cp1, cp2 });
132137
}
133138
} else {
134139
waypoints = coords;
@@ -527,10 +532,15 @@ function buildCubicPathEntries(
527532
waypoints: Array<{ x: number; y: number }>,
528533
segments: ArcPathSegment[],
529534
): string[] {
530-
const entries = [`{x: ${waypoints[0]!.x}, y: ${waypoints[0]!.y}}`];
535+
const first = waypoints[0];
536+
if (!first) return [];
537+
const entries = [`{x: ${first.x}, y: ${first.y}}`];
531538
for (let i = 0; i < segments.length; i++) {
532-
const nextWp = waypoints[i + 1]!;
533-
entries.push(...cubicControlPoints(segments[i]!, waypoints[i]!, nextWp));
539+
const seg = segments[i];
540+
const wp = waypoints[i];
541+
const nextWp = waypoints[i + 1];
542+
if (!seg || !wp || !nextWp) continue;
543+
entries.push(...cubicControlPoints(seg, wp, nextWp));
534544
entries.push(`{x: ${nextWp.x}, y: ${nextWp.y}}`);
535545
}
536546
return entries;
@@ -543,7 +553,17 @@ export function buildMotionPathObjectCode(config: {
543553
}): string {
544554
const { waypoints, segments, autoRotate } = config;
545555
const arSuffix = autoRotateSuffix(autoRotate);
546-
if (segments.some((s) => s.cp1 && s.cp2) && waypoints.length >= 2) {
556+
// GSAP's simple `path` array supports only ONE scalar `curviness` for the whole
557+
// path, so per-segment curviness can only be expressed in the cubic form (each
558+
// segment's curviness baked into its control points). Emit cubic when segments
559+
// carry explicit control points OR when their curviness values differ — the
560+
// simple branch would otherwise serialize only segments[0].curviness and drop
561+
// every other segment's curve.
562+
const hasExplicitCp = segments.some((s) => s.cp1 && s.cp2);
563+
const curvinessVaries = segments.some(
564+
(s) => (s.curviness ?? 1) !== (segments[0]?.curviness ?? 1),
565+
);
566+
if ((hasExplicitCp || curvinessVaries) && waypoints.length >= 2) {
547567
const pathStr = buildCubicPathEntries(waypoints, segments).join(", ");
548568
return `{ path: [${pathStr}], type: "cubic"${arSuffix} }`;
549569
}
Lines changed: 317 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,317 @@
1+
// fallow-ignore-file code-duplication
2+
/**
3+
* Correctness regressions for the SDK-cutover review (PR #1539).
4+
*
5+
* Each test asserts the REAL-WORLD-CORRECT result of a write op — NOT mere
6+
* agreement between the two writers. Several of these scenarios were cases where
7+
* both writers were identically wrong (so the recast-vs-acorn parity suite stayed
8+
* green); these tests pin the corrected behavior.
9+
*/
10+
import { describe, expect, it } from "vitest";
11+
import {
12+
removeKeyframeFromScript,
13+
updateKeyframeInScript,
14+
setArcPathInScript,
15+
updateArcSegmentInScript,
16+
splitAnimationsInScript,
17+
unrollDynamicAnimations,
18+
updateAnimationInScript,
19+
convertToKeyframesFromScript,
20+
} from "./gsapWriterAcorn.js";
21+
import { parseGsapScriptAcornForWrite } from "./gsapParserAcorn.js";
22+
23+
// ── #2 — findKfPropByPct must hit the CLOSEST keyframe, not first-within-2% ──
24+
25+
const KF_DENSE = `var tl = gsap.timeline({ paused: true });
26+
tl.to("#box", { keyframes: { "0%": { opacity: 0 }, "49%": { opacity: 0.4 }, "50%": { opacity: 0.5 }, "100%": { opacity: 1 } }, duration: 1 }, 0);`;
27+
const KF_DENSE_ID = "#box-to-0-visual";
28+
29+
describe("#2 — keyframe ops target the closest percentage, not first-within-tolerance", () => {
30+
it("removing 50% removes 50% and KEEPS the neighboring 49%", () => {
31+
const result = removeKeyframeFromScript(KF_DENSE, KF_DENSE_ID, 50);
32+
expect(result).not.toContain('"50%"');
33+
expect(result).toContain('"49%"');
34+
expect(result).toContain("opacity: 0.4"); // 49% body intact
35+
expect(result).toContain('"0%"');
36+
expect(result).toContain('"100%"');
37+
});
38+
39+
it("updating ~50% overwrites 50% (closest), not 49%", () => {
40+
const result = updateKeyframeInScript(KF_DENSE, KF_DENSE_ID, 51, { opacity: 0.99 });
41+
// 50% is closest to 51 (dist 1) vs 49 (dist 2) — 50% gets the new value.
42+
expect(result).toContain('"50%": { opacity: 0.99 }');
43+
// 49% must be untouched.
44+
expect(result).toContain('"49%": { opacity: 0.4 }');
45+
});
46+
});
47+
48+
// ── #4 — enableArcPath on an x/y-only tween must not produce '{}' ──
49+
50+
describe("#4 — enableArcPath on x/y-only vars yields a real motionPath", () => {
51+
const XY_ONLY = `var tl = gsap.timeline({ paused: true });
52+
tl.to("#h", { x: 100, y: 50 }, 0);`;
53+
54+
it("emits a motionPath, drops x/y, and reparses with the arc enabled", () => {
55+
const out = setArcPathInScript(XY_ONLY, "#h-to-0-position", {
56+
enabled: true,
57+
autoRotate: false,
58+
segments: [],
59+
});
60+
expect(out).toContain("motionPath");
61+
expect(out).toContain("path:");
62+
// The collision bug produced a bare '{}' (no motionPath, no x/y).
63+
expect(out).not.toMatch(/\{\s*\}/);
64+
65+
// x/y are folded into the motionPath waypoints, not left as top-level vars.
66+
const reparsed = parseGsapScriptAcornForWrite(out);
67+
const anim = reparsed?.located[0]?.animation;
68+
expect(anim?.arcPath?.enabled).toBe(true);
69+
expect("x" in (anim?.properties ?? {})).toBe(false);
70+
expect("y" in (anim?.properties ?? {})).toBe(false);
71+
});
72+
});
73+
74+
// ── #5 — split midpoint uses forward baseline (earlier tweens), not reverse ──
75+
76+
describe("#5 — split-spanning midpoint interpolates from the forward baseline", () => {
77+
// A ends at x:100 (t=0..1). B runs x:?→300 over t=1..3. Split at t=2 lands at
78+
// B's 50% point: mid = 100 + (300-100)*0.5 = 200 (NOT 150 from a 0 baseline).
79+
const TWO_TWEENS = `var tl = gsap.timeline({ paused: true });
80+
tl.to("#el", { x: 100, duration: 1 }, 0);
81+
tl.to("#el", { x: 300, duration: 2 }, 1);`;
82+
83+
it("computes midpoint x = 200 (100 + (300-100)*0.5), not 150", () => {
84+
const { script } = splitAnimationsInScript(TWO_TWEENS, {
85+
originalId: "el",
86+
newId: "el2",
87+
splitTime: 2,
88+
});
89+
expect(script).toContain("x: 200");
90+
expect(script).not.toContain("x: 150");
91+
// The new element's second half starts from the midpoint, not from 0.
92+
expect(script).toContain('tl.fromTo("#el2", { x: 200 }');
93+
});
94+
});
95+
96+
// ── #9 — unroll preserves non-target statements (tl.set) per iteration ──
97+
98+
describe("#9 — unrollDynamicAnimations keeps sibling statements in the loop body", () => {
99+
const LOOP = `var tl = gsap.timeline({ paused: true });
100+
for (let i = 0; i < 2; i++) {
101+
tl.set(items[i], { autoAlpha: 0 }, 0);
102+
tl.to(items[i], { opacity: 1, duration: 1 }, 0);
103+
}`;
104+
105+
it("the tl.set initial-state lines survive after unrolling the tl.to", () => {
106+
const parsed = parseGsapScriptAcornForWrite(LOOP);
107+
const targetId = parsed?.located.find((l) => l.animation.method === "to")?.id ?? "";
108+
const out = unrollDynamicAnimations(LOOP, targetId, [
109+
{
110+
selector: "#a",
111+
keyframes: [
112+
{ percentage: 0, properties: { opacity: 0 } },
113+
{ percentage: 100, properties: { opacity: 1 } },
114+
],
115+
},
116+
{
117+
selector: "#b",
118+
keyframes: [
119+
{ percentage: 0, properties: { opacity: 0 } },
120+
{ percentage: 100, properties: { opacity: 1 } },
121+
],
122+
},
123+
]);
124+
// Both tl.set lines must remain (one per iteration) — the blanket-overwrite
125+
// bug destroyed every non-target statement in the loop body.
126+
expect((out.match(/tl\.set\(/g) ?? []).length).toBe(2);
127+
expect(out).toContain("autoAlpha: 0");
128+
// The for-loop itself is gone (unrolled).
129+
expect(out).not.toContain("for (");
130+
// The target tween is unrolled to static selectors.
131+
expect(out).toContain('tl.to("#a"');
132+
expect(out).toContain('tl.to("#b"');
133+
});
134+
135+
it("an empty element list is a no-op, not an animation-deleting overwrite", () => {
136+
const parsed = parseGsapScriptAcornForWrite(LOOP);
137+
const targetId = parsed?.located.find((l) => l.animation.method === "to")?.id ?? "";
138+
// Empty elements has no unrolled form — overwriting the loop with zero calls
139+
// would silently delete the animation. Writer must return the script verbatim.
140+
expect(unrollDynamicAnimations(LOOP, targetId, [])).toBe(LOOP);
141+
});
142+
});
143+
144+
// ── R3 — unsafe sibling reproduction must refuse (no-op), never corrupt/drop ──
145+
146+
const TWO_EL = [
147+
{ selector: "#a", keyframes: [{ percentage: 100, properties: { opacity: 1 } }] },
148+
{ selector: "#b", keyframes: [{ percentage: 100, properties: { opacity: 1 } }] },
149+
];
150+
function targetToId(script: string): string {
151+
return (
152+
parseGsapScriptAcornForWrite(script)?.located.find((l) => l.animation.method === "to")?.id ?? ""
153+
);
154+
}
155+
function parses(src: string): boolean {
156+
try {
157+
new Function(src);
158+
return true;
159+
} catch {
160+
return false;
161+
}
162+
}
163+
164+
describe("R3 — unroll refuses (no-ops) when siblings can't be safely reproduced", () => {
165+
// R2 carried a forEach WITH a sibling tl.set to the blanket overwrite, which
166+
// dropped the tl.set (elements start visible instead of hidden). The numeric
167+
// index a `for` loop provides isn't available, so we now refuse instead.
168+
it("forEach with a sibling statement is left untouched, not flattened-and-dropped", () => {
169+
const FOREACH = `var tl = gsap.timeline({ paused: true });
170+
items.forEach((item, i) => {
171+
tl.set(item, { autoAlpha: 0 }, 0);
172+
tl.to(item, { opacity: 1, duration: 1 }, 0);
173+
});`;
174+
expect(unrollDynamicAnimations(FOREACH, targetToId(FOREACH), TWO_EL)).toBe(FOREACH);
175+
});
176+
177+
// R3 #1 — object shorthand { i }: substituting the value yields `{ 0 }` (invalid).
178+
it("object shorthand using the index refuses rather than emit invalid `{ 0 }`", () => {
179+
const SHORTHAND = `var tl = gsap.timeline({ paused: true });
180+
for (let i = 0; i < 2; i++) {
181+
tl.set(items[i], { data: { i } }, 0);
182+
tl.to(items[i], { opacity: 1, duration: 1 }, 0);
183+
}`;
184+
const out = unrollDynamicAnimations(SHORTHAND, targetToId(SHORTHAND), TWO_EL);
185+
expect(out).toBe(SHORTHAND);
186+
expect(parses(out)).toBe(true);
187+
});
188+
189+
// R3 #2 — a sibling that re-declares the index (nested for / shadowing).
190+
it("a sibling shadowing the index refuses rather than rewrite the inner binding", () => {
191+
const SHADOW = `var tl = gsap.timeline({ paused: true });
192+
for (let i = 0; i < 2; i++) {
193+
tl.set(items[i], { onStart() { for (let i = 0; i < 3; i++) log(i); } }, 0);
194+
tl.to(items[i], { opacity: 1, duration: 1 }, 0);
195+
}`;
196+
const out = unrollDynamicAnimations(SHADOW, targetToId(SHADOW), TWO_EL);
197+
expect(out).toBe(SHADOW);
198+
expect(parses(out)).toBe(true);
199+
});
200+
201+
// The safe for-loop sibling case must still unroll (regression guard).
202+
it("a plain for-loop with an items[i] sibling still unrolls and preserves it", () => {
203+
const SAFE = `var tl = gsap.timeline({ paused: true });
204+
for (let i = 0; i < 2; i++) {
205+
tl.set(items[i], { autoAlpha: 0 }, 0);
206+
tl.to(items[i], { opacity: 1, duration: 1 }, 0);
207+
}`;
208+
const out = unrollDynamicAnimations(SAFE, targetToId(SAFE), TWO_EL);
209+
expect((out.match(/tl\.set\(/g) ?? []).length).toBe(2);
210+
expect(out).not.toContain("for (");
211+
expect(parses(out)).toBe(true);
212+
});
213+
});
214+
215+
// ── R2 #5 — index substitution is AST-based: string literals are never corrupted ──
216+
217+
describe("R2 — unroll substitutes real index uses but not the index char in strings", () => {
218+
const LOOP_STR = `var tl = gsap.timeline({ paused: true });
219+
for (let i = 0; i < 2; i++) {
220+
tl.set(items[i], { id: "row-i" }, 0);
221+
tl.to(items[i], { opacity: 1, duration: 1 }, 0);
222+
}`;
223+
224+
it('rewrites items[i] per iteration but leaves the "row-i" string intact', () => {
225+
const parsed = parseGsapScriptAcornForWrite(LOOP_STR);
226+
const targetId = parsed?.located.find((l) => l.animation.method === "to")?.id ?? "";
227+
const out = unrollDynamicAnimations(LOOP_STR, targetId, [
228+
{ selector: "#a", keyframes: [{ percentage: 100, properties: { opacity: 1 } }] },
229+
{ selector: "#b", keyframes: [{ percentage: 100, properties: { opacity: 1 } }] },
230+
]);
231+
// Real uses of the index are substituted…
232+
expect(out).toContain("items[0]");
233+
expect(out).toContain("items[1]");
234+
// …but the literal "row-i" is untouched (the regex bug rewrote it to "row-0").
235+
expect(out).toContain('"row-i"');
236+
expect(out).not.toContain('"row-0"');
237+
});
238+
});
239+
240+
// ── #10 — per-segment curviness survives serialization ──
241+
242+
describe("#10 — updateArcSegment on a non-first segment reflects its curviness", () => {
243+
// 3-waypoint arc, uniform curviness 1.5 → change segment 1 to curviness 3.
244+
const ARC = `var tl = gsap.timeline({ paused: true });
245+
tl.to("#h", { motionPath: { path: [{x: 0, y: 0}, {x: 100, y: 50}, {x: 200, y: 0}], curviness: 1.5 }, duration: 1 }, 0);`;
246+
247+
it("does not drop the second segment's curve (no longer serializes only segments[0])", () => {
248+
const parsed = parseGsapScriptAcornForWrite(ARC);
249+
const id = parsed?.located[0]?.id ?? "";
250+
const out = updateArcSegmentInScript(ARC, id, 1, { curviness: 3 });
251+
252+
// With differing per-segment curviness, the only representation that carries
253+
// both is the cubic form. The simple form (which only emits one scalar
254+
// curviness) would silently drop segment 1's change.
255+
expect(out).toContain('type: "cubic"');
256+
257+
// Compare against the SAME-shape arc left at uniform curviness 1.5: the
258+
// segment-1 control points must DIFFER, proving curviness 3 took effect.
259+
const uniformOut = updateArcSegmentInScript(ARC, id, 1, { curviness: 1.5 });
260+
expect(out).not.toBe(uniformOut);
261+
});
262+
});
263+
264+
// ── #11 — disableArcPath recovers NEGATIVE destination coordinates ──
265+
266+
describe("#11 — disableArcPath restores negative waypoint coords", () => {
267+
it("restores x:-120, y:-40 on the flattened tween", () => {
268+
const XY_NEG = `var tl = gsap.timeline({ paused: true });
269+
tl.to("#h", { x: -120, y: -40, duration: 1 }, 0);`;
270+
const enabled = setArcPathInScript(XY_NEG, "#h-to-0-position", {
271+
enabled: true,
272+
autoRotate: false,
273+
segments: [],
274+
});
275+
const reEnabled = parseGsapScriptAcornForWrite(enabled);
276+
const id = reEnabled?.located[0]?.id ?? "";
277+
const disabled = setArcPathInScript(enabled, id, {
278+
enabled: false,
279+
autoRotate: false,
280+
segments: [],
281+
});
282+
// The negative destination must come back — the UnaryExpression bug lost it.
283+
expect(disabled).toContain("x: -120");
284+
expect(disabled).toContain("y: -40");
285+
expect(disabled).not.toContain("motionPath");
286+
});
287+
});
288+
289+
// ── #7 — updating ease on a keyframe tween routes to easeEach, not top-level ──
290+
291+
describe("#7 — ease update on a keyframe tween targets keyframes.easeEach", () => {
292+
const KF = `var tl = gsap.timeline({ paused: true });
293+
tl.to(".a", { keyframes: { "0%": { x: 0 }, "100%": { x: 100 } }, duration: 1, ease: "none" }, 0);`;
294+
295+
it("writes easeEach (per-keyframe), not a no-op top-level ease", () => {
296+
const id = parseGsapScriptAcornForWrite(KF)?.located[0]?.id ?? "";
297+
const out = updateAnimationInScript(KF, id, { ease: "power2.inOut" });
298+
expect(out).toContain('easeEach: "power2.inOut"');
299+
// The original top-level `ease: "none"` is untouched (no second top-level ease).
300+
expect((out.match(/ease: "power2.inOut"/g) ?? []).length).toBe(0);
301+
});
302+
});
303+
304+
// ── #8 — convertToKeyframes preserves builtin vars like `delay` ──
305+
306+
describe("#8 — convertToKeyframes keeps delay (was dropped, shifting start time)", () => {
307+
const DELAY = `var tl = gsap.timeline({ paused: true });
308+
tl.to(".a", { x: 100, duration: 1, delay: 0.3 }, 0);`;
309+
310+
it("preserves delay on the converted vars object", () => {
311+
const id = parseGsapScriptAcornForWrite(DELAY)?.located[0]?.id ?? "";
312+
const out = convertToKeyframesFromScript(DELAY, id);
313+
expect(out).toContain("keyframes:");
314+
expect(out).toContain("delay: 0.3"); // was lost → tween started 0.3s early
315+
expect(out).toContain("duration: 1");
316+
});
317+
});

0 commit comments

Comments
 (0)