Skip to content

Commit 8c7672f

Browse files
fix(parsers): splitting a clip keeps keyframes the parser cannot read (#5044)
* fix(parsers): splitting a clip keeps keyframes the parser cannot read * refactor(parsers): keyframed split branch in its own function * fix(parsers): no writer rewrites keyframes it cannot read; a split reports an unknown start * fix(parsers): recast property edits keep keyframes they cannot read * fix(parsers): keyframe edits refuse unreadable object keyframes at the one locator * style(parsers): shorten the unroll safety comment * fix(parsers): a split never invents a start value from unreadable keyframes * fix(parsers): object keyframe steps that are named, spread or number-keyed are unreadable * fix(parsers): array steps, getters, stray channels and unresolved array values read as unreadable keyframes
1 parent 60575dc commit 8c7672f

7 files changed

Lines changed: 226 additions & 31 deletions

File tree

‎packages/parsers/src/gsapParser.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
safeJsKey as safeKey,
2323
resolveConversionProps,
2424
mergePercentageKeyframes,
25+
authorsKeyframes,
2526
} from "./gsapSerialize";
2627

2728
export type {
@@ -1551,7 +1552,7 @@ export function updateAnimationInScript(
15511552
return script;
15521553
}
15531554
const target = parsed.located.find((l) => l.id === animationId);
1554-
if (!target) return script;
1555+
if (!target || (updates.properties && target.animation.hasUnresolvedKeyframes)) return script;
15551556
applyUpdatesToCall(target.call, updates);
15561557
return recast.print(parsed.ast).code;
15571558
}
@@ -1943,14 +1944,14 @@ export function splitAnimationsInScript(
19431944
const dur = anim.duration ?? 0;
19441945
const animEnd = pos + dur;
19451946

1946-
if (anim.keyframes) {
1947+
if (authorsKeyframes(anim)) {
19471948
if (pos >= opts.splitTime) {
19481949
result = updateAnimationSelector(result, anim.id, newSelector);
19491950
} else if (animEnd > opts.splitTime) {
19501951
// Spanning keyframes can't be correctly split without renormalizing
19511952
// percentages and durations — leave on original, warn the caller.
19521953
skippedSelectors.push(`${originalSelector} (keyframes spanning split)`);
1953-
const kfs = anim.keyframes.keyframes;
1954+
const kfs = anim.keyframes?.keyframes ?? [];
19541955
for (const kf of kfs) {
19551956
const kfTime = pos + (kf.percentage / 100) * dur;
19561957
if (kfTime <= opts.splitTime) {
@@ -1961,7 +1962,10 @@ export function splitAnimationsInScript(
19611962
}
19621963
} else {
19631964
// Entirely before split — extract final keyframe properties
1964-
const kfs = anim.keyframes.keyframes;
1965+
if (anim.hasUnresolvedKeyframes) {
1966+
skippedSelectors.push(`${originalSelector} (unreadable keyframes before split)`);
1967+
}
1968+
const kfs = anim.keyframes?.keyframes ?? [];
19651969
if (kfs.length > 0) {
19661970
for (const [k, v] of Object.entries(kfs[kfs.length - 1]!.properties)) {
19671971
inheritedProps[k] = v;
@@ -2717,7 +2721,7 @@ export function convertToKeyframesInScript(
27172721
if (!loc) return script;
27182722

27192723
const anim = loc.target.animation;
2720-
if (anim.keyframes) return script;
2724+
if (authorsKeyframes(anim)) return script;
27212725

27222726
const { fromProps, toProps } = resolveConversionProps(anim, resolvedFromValues);
27232727
const varsArg = loc.target.call.varsArg;
@@ -3266,6 +3270,7 @@ export function splitIntoPropertyGroups(
32663270
if (!loc) return { script, ids: [animationId] };
32673271

32683272
const anim = loc.target.animation;
3273+
if (anim.hasUnresolvedKeyframes) return { script, ids: [animationId] };
32693274

32703275
// Collect the properties to partition. For keyframed tweens, gather the
32713276
// union of all properties across all keyframes. For flat tweens, use the

‎packages/parsers/src/gsapParserAcorn.ts‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ function parsePercentageKeyframes(
10051005
node: any,
10061006
scope: ScopeBindings,
10071007
source: string,
1008-
): GsapKeyframesData {
1008+
): GsapKeyframesData | undefined {
10091009
const keyframes: GsapPercentageKeyframe[] = [];
10101010
let ease: string | undefined;
10111011
let easeEach: string | undefined;
@@ -1017,6 +1017,7 @@ function parsePercentageKeyframes(
10171017

10181018
const pctMatch = PERCENTAGE_KEY_RE.exec(key);
10191019
if (pctMatch) {
1020+
if (!isPlainStep(prop.value)) return undefined;
10201021
const percentage = Number.parseFloat(pctMatch[1] ?? "0");
10211022
const record = objectExpressionToRecord(prop.value, scope, source);
10221023
const properties: Record<string, number | string> = {};
@@ -1041,6 +1042,8 @@ function parsePercentageKeyframes(
10411042
ease = tryResolveStringProp(prop.value, scope, source) ?? ease;
10421043
} else if (key === "easeEach") {
10431044
easeEach = tryResolveStringProp(prop.value, scope, source) ?? easeEach;
1045+
} else {
1046+
return undefined;
10441047
}
10451048
}
10461049

@@ -1073,8 +1076,20 @@ function computeKeyframesTotalDuration(
10731076
return getObjectArrayKeyframeTiming(durations)?.totalDuration;
10741077
}
10751078

1076-
const isFullyReadableStep = (el: any) =>
1077-
el?.type === "ObjectExpression" && !el.properties.some((p: any) => p.type === "SpreadElement");
1079+
function staticKeyName(prop: any): string | undefined {
1080+
if (prop.type !== "ObjectProperty" && prop.type !== "Property") return undefined;
1081+
if (prop.method || (prop.kind && prop.kind !== "init")) return undefined;
1082+
const key = prop.computed
1083+
? prop.key?.type === "Literal" && prop.key.value
1084+
: (prop.key?.name ?? prop.key?.value);
1085+
return typeof key === "string" ? key : undefined;
1086+
}
1087+
1088+
const isPlainObject = (node: any) =>
1089+
node?.type === "ObjectExpression" &&
1090+
(node.properties ?? []).every((p: any) => staticKeyName(p) !== undefined);
1091+
1092+
const isPlainStep = (node: any) => node?.type === "Literal" || isPlainObject(node);
10781093

10791094
// fallow-ignore-next-line complexity
10801095
function parseObjectArrayKeyframes(
@@ -1090,7 +1105,7 @@ function parseObjectArrayKeyframes(
10901105
}> = [];
10911106

10921107
for (const el of elements) {
1093-
if (!isFullyReadableStep(el)) return undefined;
1108+
if (!isPlainObject(el)) return undefined;
10941109
const record = objectExpressionToRecord(el, scope, source);
10951110
const properties: Record<string, number | string> = {};
10961111
let duration: unknown;
@@ -1122,7 +1137,7 @@ function parseObjectArrayKeyframes(
11221137
}
11231138

11241139
// fallow-ignore-next-line complexity
1125-
function parseSimpleArrayKeyframes(node: any, scope: ScopeBindings): GsapKeyframesData {
1140+
function parseSimpleArrayKeyframes(node: any, scope: ScopeBindings): GsapKeyframesData | undefined {
11261141
const arrayProps: Map<string, (number | string)[]> = new Map();
11271142
let ease: string | undefined;
11281143
let easeEach: string | undefined;
@@ -1136,9 +1151,8 @@ function parseSimpleArrayKeyframes(node: any, scope: ScopeBindings): GsapKeyfram
11361151
const values: (number | string)[] = [];
11371152
for (const el of prop.value.elements ?? []) {
11381153
const val = resolveNode(el, scope);
1139-
if (typeof val === "number" || typeof val === "string") {
1140-
values.push(val);
1141-
}
1154+
if (typeof val !== "number" && typeof val !== "string") return undefined;
1155+
values.push(val);
11421156
}
11431157
if (values.length > 0) arrayProps.set(key, values);
11441158
} else if (key === "ease") {
@@ -1183,6 +1197,7 @@ function parseKeyframesNode(
11831197
if (node.type !== "ObjectExpression") return undefined;
11841198

11851199
const props = node.properties ?? [];
1200+
if (!props.every((p: any) => staticKeyName(p) !== undefined)) return undefined;
11861201
let hasPercentageKey = false;
11871202
let hasArrayValue = false;
11881203

‎packages/parsers/src/gsapSerialize.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ export function editabilityForProvenance(provenance?: GsapProvenance): KeyframeE
4747
return "unroll";
4848
}
4949

50+
export function authorsKeyframes(anim: GsapAnimation): boolean {
51+
return anim.keyframes !== undefined || anim.hasUnresolvedKeyframes === true;
52+
}
53+
5054
export interface GsapAnimation {
5155
id: string;
5256
targetSelector: string;

‎packages/parsers/src/gsapUnroll.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,13 @@ fade("#a", 1);`;
228228
expect(unrollComputedTimeline(script)).toBe(script);
229229
});
230230

231+
it("leaves a loop as authored when a tween's keyframes cannot be read", () => {
232+
const script = `const tl = gsap.timeline();
233+
for (let i = 0; i < 2; i++) { tl.to("#x", { duration: 1, keyframes: [{ x: 0 }, { x: 100, runBackwards: true }] }, i); }`;
234+
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
235+
expect(unrollComputedTimeline(script)).toBe(script);
236+
});
237+
231238
it("leaves a loop as authored when its body sets state outside the timeline", () => {
232239
const script = `const tl = gsap.timeline();
233240
for (let i = 0; i < 2; i++) { gsap.set("#x", { opacity: 0 }); tl.to("#x", { opacity: 1, duration: 1 }, i); }`;

‎packages/parsers/src/gsapUnroll.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,14 +227,18 @@ function declaredFunctions(stmt: Node): Array<[string, Node[]]> {
227227
const collectHelperBodies = (statements: Node[]): Map<string, Node[]> =>
228228
new Map(statements.flatMap(declaredFunctions));
229229

230-
/** Statements stay as authored when literal tweens cannot encode their timing, or they do more than add tweens. */
230+
/** Left as authored when literal tweens can't encode the timing or keyframes, or it does more than add tweens. */
231231
function dropStatementsUnsafeToUnroll(
232232
byStatement: Map<Node, GsapAnimation[]>,
233233
ctx: UnrollScope,
234234
): void {
235235
for (const [stmt, anims] of byStatement) {
236236
const unknownTiming = anims.some(
237-
(a) => a.durationUnresolved || a.resolvedStart === undefined || a.hasUnresolvedSelector,
237+
(a) =>
238+
a.durationUnresolved ||
239+
a.resolvedStart === undefined ||
240+
a.hasUnresolvedSelector ||
241+
a.hasUnresolvedKeyframes,
238242
);
239243
const bodyStmts = unknownTiming ? null : bodyOf(stmt, ctx.helpers);
240244
if (bodyStmts === null || !onlyAddsTweens(bodyStmts, ctx)) byStatement.delete(stmt);

‎packages/parsers/src/gsapWriter.parity.test.ts‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
materializeKeyframesInScript as materializeRecast,
1919
splitIntoPropertyGroups as splitGroupsRecast,
2020
splitAnimationsInScript as splitAnimsRecast,
21+
updateAnimationInScript as updateAnimRecast,
2122
setArcPathInScript as setArcRecast,
2223
updateArcSegmentInScript as updateArcSegmentRecast,
2324
removeArcPathFromScript as removeArcRecast,
@@ -55,6 +56,7 @@ import {
5556
resizeKeyframedTweenInScript as resizeKeyframedTweenAcorn,
5657
addAnimationWithKeyframesToScript as addWithKfAcorn,
5758
removeAnimationFromScript as removeAnimAcorn,
59+
updateAnimationInScript as updateAnimAcorn,
5860
shiftPositionsInScript as shiftAcorn,
5961
scalePositionsInScript as scaleAcorn,
6062
retimeClipTweensInScript,
@@ -570,6 +572,150 @@ describe("parity: splitAnimationsInScript (recast vs acorn)", () => {
570572
});
571573
});
572574

575+
describe("splitAnimationsInScript keeps keyframes the parser cannot read", () => {
576+
const STEP_SHAPES = [
577+
["a step with a flag", "", "[{ x: 0 }, { x: 100, runBackwards: true }]"],
578+
["a spread step", "const base = { x: 0 };", "[{ ...base }, { x: 100 }]"],
579+
["a named step", "const last = { x: 100 };", "[{ x: 0 }, last]"],
580+
["keyframes from a call", "", "steps()"],
581+
] as const;
582+
const opts = { originalId: "a", newId: "a-2", elementStart: 0, elementDuration: 4 };
583+
const writers = [
584+
["recast", splitAnimsRecast],
585+
["acorn", splitAnimsAcorn],
586+
] as const;
587+
588+
for (const [writer, split] of writers) {
589+
for (const [shape, decl, keyframes] of STEP_SHAPES) {
590+
const script = `const tl = gsap.timeline({ paused: true });
591+
${decl}
592+
tl.to("#a", { duration: 2, keyframes: ${keyframes} }, 0);`;
593+
594+
it(`${writer}: leaves ${shape} spanning the split as authored`, () => {
595+
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
596+
const result = split(script, { ...opts, splitTime: 1 });
597+
expect(result.script).toContain(`tl.to("#a", { duration: 2, keyframes: ${keyframes} }, 0)`);
598+
expect(result.skippedSelectors).toContain("#a (keyframes spanning split)");
599+
});
600+
601+
it(`${writer}: moves ${shape} after the split whole`, () => {
602+
const result = split(script, { ...opts, splitTime: 0 });
603+
expect(result.script).toContain(
604+
`tl.to("#a-2", { duration: 2, keyframes: ${keyframes} }, 0)`,
605+
);
606+
});
607+
}
608+
}
609+
});
610+
611+
describe("writers leave keyframes they cannot read as authored", () => {
612+
const UNREADABLE = [
613+
["a step with a flag", "[{ x: 0 }, { x: 100, runBackwards: true }]"],
614+
["keyframes from a call", "steps()"],
615+
] as const;
616+
const opts = { originalId: "a", newId: "a-2", elementStart: 0, elementDuration: 4 };
617+
618+
for (const [shape, keyframes] of UNREADABLE) {
619+
const script = `const tl = gsap.timeline({ paused: true });
620+
tl.to("#a", { opacity: 1, x: 5, duration: 1, keyframes: ${keyframes} }, 0);`;
621+
const id = () => acornId(script);
622+
623+
for (const [writer, split] of [
624+
["recast", splitAnimsRecast],
625+
["acorn", splitAnimsAcorn],
626+
] as const) {
627+
if (writer === "recast" && keyframes !== "steps()") continue;
628+
it(`${writer}: a split after ${shape} reports that the new clip's start is unknown`, () => {
629+
const result = split(`${script}\ntl.to("#a", { duration: 2, x: 200 }, 1);`, {
630+
...opts,
631+
splitTime: 2,
632+
});
633+
expect(result.skippedSelectors).toContain("#a (unreadable keyframes before split)");
634+
});
635+
}
636+
637+
it.each([
638+
["recast convert to keyframes", () => convertRecast(script, id())],
639+
["acorn convert to keyframes", () => convertAcorn(script, id())],
640+
["recast split into property groups", () => splitGroupsRecast(script, id()).script],
641+
["acorn split into property groups", () => splitGroupsAcorn(script, id()).script],
642+
["acorn add keyframe", () => addKeyframeAcorn(script, id(), 50, { x: 10 })],
643+
["acorn update keyframe", () => updateKeyframeAcorn(script, id(), 100, { x: 10 })],
644+
["acorn remove keyframe", () => removeKeyframeAcorn(script, id(), 100)],
645+
["acorn move keyframe", () => moveKeyframeAcorn(script, id(), 100, 80)],
646+
["acorn resize keyframed tween", () => resizeKeyframedTweenAcorn(script, id(), 0, 2, [])],
647+
[
648+
"acorn property edit",
649+
() => updateAnimAcorn(script, id(), { properties: { opacity: 0.5 } }),
650+
],
651+
])(`%s leaves ${shape} unchanged`, (_name, write) => {
652+
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
653+
expect(write()).toBe(script);
654+
});
655+
}
656+
657+
it.each(["{ x: v }", "{ [v]: { x: 1 } }", "{ ...o }"])(
658+
"acorn add and resize leave object keyframes %s unchanged",
659+
(keyframes) => {
660+
const script = `const tl = gsap.timeline({ paused: true });
661+
tl.to("#a", { duration: 1, keyframes: ${keyframes} }, 0);`;
662+
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
663+
expect(addKeyframeAcorn(script, acornId(script), 50, { x: 5 })).toBe(script);
664+
expect(resizeKeyframedTweenAcorn(script, acornId(script), 0, 2, [])).toBe(script);
665+
},
666+
);
667+
668+
it.each([
669+
["recast", splitAnimsRecast],
670+
["acorn", splitAnimsAcorn],
671+
] as const)("%s: a split across unreadable keyframes writes no start value", (_writer, split) => {
672+
const script = `const tl = gsap.timeline({ paused: true });
673+
tl.to("#a", { opacity: 0.5, duration: 2, keyframes: steps() }, 0);`;
674+
const result = split(script, { ...opts, splitTime: 1 });
675+
expect(result.script).not.toContain('"#a-2"');
676+
expect(result.skippedSelectors).toContain("#a (keyframes spanning split)");
677+
});
678+
679+
it("treats keyframes with a computed percentage key as unreadable", () => {
680+
const script = `const tl = gsap.timeline({ paused: true });
681+
tl.to("#a", { duration: 1, keyframes: { [k]: { x: 10 }, "100%": { x: 50 } } }, 0);`;
682+
const id = acornId(script);
683+
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
684+
expect(updateAnimAcorn(script, id, { properties: { opacity: 0.5 } })).toBe(script);
685+
expect(removeKeyframeAcorn(script, id, 100)).toBe(script);
686+
});
687+
688+
it.each([
689+
["a named step", '{ "0%": { x: 0 }, "100%": last }'],
690+
["a spread inside a step", '{ "0%": { x: 0 }, "100%": { ...base, x: 10 } }'],
691+
["a numeric key", '{ "0%": { x: 0 }, [50]: { x: 5 }, "100%": { x: 10 } }'],
692+
["a template key in an array step", "[{ x: 0 }, { x: 10, [`y`]: 5 }]"],
693+
["a variable key in an array step", "[{ x: 0 }, { x: 10, [k]: 5 }]"],
694+
["a getter in a step", '{ "0%": { x: 0 }, "100%": { get x() { return 10; } } }'],
695+
["a method in an array step", "[{ x: 0 }, { x: 10, y() {} }]"],
696+
["an array channel beside % steps", '{ "0%": { x: 0 }, "100%": { x: 10 }, y: [0, 5] }'],
697+
["an unresolved array value", "{ x: [0, 10, v] }"],
698+
["a spread in an array channel", "{ x: [0, ...rest] }"],
699+
["a hole in an array channel", "{ x: [0, , 10] }"],
700+
])(
701+
"treats keyframes with %s as unreadable, so drags and deletes keep them",
702+
(_shape, keyframes) => {
703+
const script = `const tl = gsap.timeline({ paused: true });
704+
tl.to("#a", { duration: 2, keyframes: ${keyframes} }, 0);`;
705+
const id = acornId(script);
706+
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
707+
expect(moveKeyframeAcorn(script, id, 100, 80)).toBe(script);
708+
expect(removeKeyframeAcorn(script, id, 0)).toBe(script);
709+
},
710+
);
711+
712+
it("recast property edit leaves keyframes from a call unchanged", () => {
713+
const script = `const tl = gsap.timeline({ paused: true });
714+
tl.to("#a", { x: 5, duration: 1, keyframes: steps() }, 0);`;
715+
expect(updateAnimRecast(script, acornId(script), { properties: { x: 9 } })).toBe(script);
716+
});
717+
});
718+
573719
// ─── arc path parity ──────────────────────────────────────────────────────────
574720

575721
const ARC_FLAT_SCRIPT = `

0 commit comments

Comments
 (0)