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
This commit is contained in:
Miguel Ángel
2026-10-05 07:36:55 +00:00
committed by GitHub
parent 60575dc0d1
commit 8c7672f105
7 changed files with 226 additions and 31 deletions
+10 -5
View File
@@ -22,6 +22,7 @@ import {
safeJsKey as safeKey,
resolveConversionProps,
mergePercentageKeyframes,
authorsKeyframes,
} from "./gsapSerialize";
export type {
@@ -1551,7 +1552,7 @@ export function updateAnimationInScript(
return script;
}
const target = parsed.located.find((l) => l.id === animationId);
if (!target) return script;
if (!target || (updates.properties && target.animation.hasUnresolvedKeyframes)) return script;
applyUpdatesToCall(target.call, updates);
return recast.print(parsed.ast).code;
}
@@ -1943,14 +1944,14 @@ export function splitAnimationsInScript(
const dur = anim.duration ?? 0;
const animEnd = pos + dur;
if (anim.keyframes) {
if (authorsKeyframes(anim)) {
if (pos >= opts.splitTime) {
result = updateAnimationSelector(result, anim.id, newSelector);
} else if (animEnd > opts.splitTime) {
// Spanning keyframes can't be correctly split without renormalizing
// percentages and durations — leave on original, warn the caller.
skippedSelectors.push(`${originalSelector} (keyframes spanning split)`);
const kfs = anim.keyframes.keyframes;
const kfs = anim.keyframes?.keyframes ?? [];
for (const kf of kfs) {
const kfTime = pos + (kf.percentage / 100) * dur;
if (kfTime <= opts.splitTime) {
@@ -1961,7 +1962,10 @@ export function splitAnimationsInScript(
}
} else {
// Entirely before split — extract final keyframe properties
const kfs = anim.keyframes.keyframes;
if (anim.hasUnresolvedKeyframes) {
skippedSelectors.push(`${originalSelector} (unreadable keyframes before split)`);
}
const kfs = anim.keyframes?.keyframes ?? [];
if (kfs.length > 0) {
for (const [k, v] of Object.entries(kfs[kfs.length - 1]!.properties)) {
inheritedProps[k] = v;
@@ -2717,7 +2721,7 @@ export function convertToKeyframesInScript(
if (!loc) return script;
const anim = loc.target.animation;
if (anim.keyframes) return script;
if (authorsKeyframes(anim)) return script;
const { fromProps, toProps } = resolveConversionProps(anim, resolvedFromValues);
const varsArg = loc.target.call.varsArg;
@@ -3266,6 +3270,7 @@ export function splitIntoPropertyGroups(
if (!loc) return { script, ids: [animationId] };
const anim = loc.target.animation;
if (anim.hasUnresolvedKeyframes) return { script, ids: [animationId] };
// Collect the properties to partition. For keyframed tweens, gather the
// union of all properties across all keyframes. For flat tweens, use the
+23 -8
View File
@@ -1005,7 +1005,7 @@ function parsePercentageKeyframes(
node: any,
scope: ScopeBindings,
source: string,
): GsapKeyframesData {
): GsapKeyframesData | undefined {
const keyframes: GsapPercentageKeyframe[] = [];
let ease: string | undefined;
let easeEach: string | undefined;
@@ -1017,6 +1017,7 @@ function parsePercentageKeyframes(
const pctMatch = PERCENTAGE_KEY_RE.exec(key);
if (pctMatch) {
if (!isPlainStep(prop.value)) return undefined;
const percentage = Number.parseFloat(pctMatch[1] ?? "0");
const record = objectExpressionToRecord(prop.value, scope, source);
const properties: Record<string, number | string> = {};
@@ -1041,6 +1042,8 @@ function parsePercentageKeyframes(
ease = tryResolveStringProp(prop.value, scope, source) ?? ease;
} else if (key === "easeEach") {
easeEach = tryResolveStringProp(prop.value, scope, source) ?? easeEach;
} else {
return undefined;
}
}
@@ -1073,8 +1076,20 @@ function computeKeyframesTotalDuration(
return getObjectArrayKeyframeTiming(durations)?.totalDuration;
}
const isFullyReadableStep = (el: any) =>
el?.type === "ObjectExpression" && !el.properties.some((p: any) => p.type === "SpreadElement");
function staticKeyName(prop: any): string | undefined {
if (prop.type !== "ObjectProperty" && prop.type !== "Property") return undefined;
if (prop.method || (prop.kind && prop.kind !== "init")) return undefined;
const key = prop.computed
? prop.key?.type === "Literal" && prop.key.value
: (prop.key?.name ?? prop.key?.value);
return typeof key === "string" ? key : undefined;
}
const isPlainObject = (node: any) =>
node?.type === "ObjectExpression" &&
(node.properties ?? []).every((p: any) => staticKeyName(p) !== undefined);
const isPlainStep = (node: any) => node?.type === "Literal" || isPlainObject(node);
// fallow-ignore-next-line complexity
function parseObjectArrayKeyframes(
@@ -1090,7 +1105,7 @@ function parseObjectArrayKeyframes(
}> = [];
for (const el of elements) {
if (!isFullyReadableStep(el)) return undefined;
if (!isPlainObject(el)) return undefined;
const record = objectExpressionToRecord(el, scope, source);
const properties: Record<string, number | string> = {};
let duration: unknown;
@@ -1122,7 +1137,7 @@ function parseObjectArrayKeyframes(
}
// fallow-ignore-next-line complexity
function parseSimpleArrayKeyframes(node: any, scope: ScopeBindings): GsapKeyframesData {
function parseSimpleArrayKeyframes(node: any, scope: ScopeBindings): GsapKeyframesData | undefined {
const arrayProps: Map<string, (number | string)[]> = new Map();
let ease: string | undefined;
let easeEach: string | undefined;
@@ -1136,9 +1151,8 @@ function parseSimpleArrayKeyframes(node: any, scope: ScopeBindings): GsapKeyfram
const values: (number | string)[] = [];
for (const el of prop.value.elements ?? []) {
const val = resolveNode(el, scope);
if (typeof val === "number" || typeof val === "string") {
values.push(val);
}
if (typeof val !== "number" && typeof val !== "string") return undefined;
values.push(val);
}
if (values.length > 0) arrayProps.set(key, values);
} else if (key === "ease") {
@@ -1183,6 +1197,7 @@ function parseKeyframesNode(
if (node.type !== "ObjectExpression") return undefined;
const props = node.properties ?? [];
if (!props.every((p: any) => staticKeyName(p) !== undefined)) return undefined;
let hasPercentageKey = false;
let hasArrayValue = false;
+4
View File
@@ -47,6 +47,10 @@ export function editabilityForProvenance(provenance?: GsapProvenance): KeyframeE
return "unroll";
}
export function authorsKeyframes(anim: GsapAnimation): boolean {
return anim.keyframes !== undefined || anim.hasUnresolvedKeyframes === true;
}
export interface GsapAnimation {
id: string;
targetSelector: string;
+7
View File
@@ -228,6 +228,13 @@ fade("#a", 1);`;
expect(unrollComputedTimeline(script)).toBe(script);
});
it("leaves a loop as authored when a tween's keyframes cannot be read", () => {
const script = `const tl = gsap.timeline();
for (let i = 0; i < 2; i++) { tl.to("#x", { duration: 1, keyframes: [{ x: 0 }, { x: 100, runBackwards: true }] }, i); }`;
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
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); }`;
+6 -2
View File
@@ -227,14 +227,18 @@ function declaredFunctions(stmt: Node): Array<[string, Node[]]> {
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. */
/** Left as authored when literal tweens can't encode the timing or keyframes, or it does 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,
(a) =>
a.durationUnresolved ||
a.resolvedStart === undefined ||
a.hasUnresolvedSelector ||
a.hasUnresolvedKeyframes,
);
const bodyStmts = unknownTiming ? null : bodyOf(stmt, ctx.helpers);
if (bodyStmts === null || !onlyAddsTweens(bodyStmts, ctx)) byStatement.delete(stmt);
@@ -18,6 +18,7 @@ import {
materializeKeyframesInScript as materializeRecast,
splitIntoPropertyGroups as splitGroupsRecast,
splitAnimationsInScript as splitAnimsRecast,
updateAnimationInScript as updateAnimRecast,
setArcPathInScript as setArcRecast,
updateArcSegmentInScript as updateArcSegmentRecast,
removeArcPathFromScript as removeArcRecast,
@@ -55,6 +56,7 @@ import {
resizeKeyframedTweenInScript as resizeKeyframedTweenAcorn,
addAnimationWithKeyframesToScript as addWithKfAcorn,
removeAnimationFromScript as removeAnimAcorn,
updateAnimationInScript as updateAnimAcorn,
shiftPositionsInScript as shiftAcorn,
scalePositionsInScript as scaleAcorn,
retimeClipTweensInScript,
@@ -570,6 +572,150 @@ describe("parity: splitAnimationsInScript (recast vs acorn)", () => {
});
});
describe("splitAnimationsInScript keeps keyframes the parser cannot read", () => {
const STEP_SHAPES = [
["a step with a flag", "", "[{ x: 0 }, { x: 100, runBackwards: true }]"],
["a spread step", "const base = { x: 0 };", "[{ ...base }, { x: 100 }]"],
["a named step", "const last = { x: 100 };", "[{ x: 0 }, last]"],
["keyframes from a call", "", "steps()"],
] as const;
const opts = { originalId: "a", newId: "a-2", elementStart: 0, elementDuration: 4 };
const writers = [
["recast", splitAnimsRecast],
["acorn", splitAnimsAcorn],
] as const;
for (const [writer, split] of writers) {
for (const [shape, decl, keyframes] of STEP_SHAPES) {
const script = `const tl = gsap.timeline({ paused: true });
${decl}
tl.to("#a", { duration: 2, keyframes: ${keyframes} }, 0);`;
it(`${writer}: leaves ${shape} spanning the split as authored`, () => {
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
const result = split(script, { ...opts, splitTime: 1 });
expect(result.script).toContain(`tl.to("#a", { duration: 2, keyframes: ${keyframes} }, 0)`);
expect(result.skippedSelectors).toContain("#a (keyframes spanning split)");
});
it(`${writer}: moves ${shape} after the split whole`, () => {
const result = split(script, { ...opts, splitTime: 0 });
expect(result.script).toContain(
`tl.to("#a-2", { duration: 2, keyframes: ${keyframes} }, 0)`,
);
});
}
}
});
describe("writers leave keyframes they cannot read as authored", () => {
const UNREADABLE = [
["a step with a flag", "[{ x: 0 }, { x: 100, runBackwards: true }]"],
["keyframes from a call", "steps()"],
] as const;
const opts = { originalId: "a", newId: "a-2", elementStart: 0, elementDuration: 4 };
for (const [shape, keyframes] of UNREADABLE) {
const script = `const tl = gsap.timeline({ paused: true });
tl.to("#a", { opacity: 1, x: 5, duration: 1, keyframes: ${keyframes} }, 0);`;
const id = () => acornId(script);
for (const [writer, split] of [
["recast", splitAnimsRecast],
["acorn", splitAnimsAcorn],
] as const) {
if (writer === "recast" && keyframes !== "steps()") continue;
it(`${writer}: a split after ${shape} reports that the new clip's start is unknown`, () => {
const result = split(`${script}\ntl.to("#a", { duration: 2, x: 200 }, 1);`, {
...opts,
splitTime: 2,
});
expect(result.skippedSelectors).toContain("#a (unreadable keyframes before split)");
});
}
it.each([
["recast convert to keyframes", () => convertRecast(script, id())],
["acorn convert to keyframes", () => convertAcorn(script, id())],
["recast split into property groups", () => splitGroupsRecast(script, id()).script],
["acorn split into property groups", () => splitGroupsAcorn(script, id()).script],
["acorn add keyframe", () => addKeyframeAcorn(script, id(), 50, { x: 10 })],
["acorn update keyframe", () => updateKeyframeAcorn(script, id(), 100, { x: 10 })],
["acorn remove keyframe", () => removeKeyframeAcorn(script, id(), 100)],
["acorn move keyframe", () => moveKeyframeAcorn(script, id(), 100, 80)],
["acorn resize keyframed tween", () => resizeKeyframedTweenAcorn(script, id(), 0, 2, [])],
[
"acorn property edit",
() => updateAnimAcorn(script, id(), { properties: { opacity: 0.5 } }),
],
])(`%s leaves ${shape} unchanged`, (_name, write) => {
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
expect(write()).toBe(script);
});
}
it.each(["{ x: v }", "{ [v]: { x: 1 } }", "{ ...o }"])(
"acorn add and resize leave object keyframes %s unchanged",
(keyframes) => {
const script = `const tl = gsap.timeline({ paused: true });
tl.to("#a", { duration: 1, keyframes: ${keyframes} }, 0);`;
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
expect(addKeyframeAcorn(script, acornId(script), 50, { x: 5 })).toBe(script);
expect(resizeKeyframedTweenAcorn(script, acornId(script), 0, 2, [])).toBe(script);
},
);
it.each([
["recast", splitAnimsRecast],
["acorn", splitAnimsAcorn],
] as const)("%s: a split across unreadable keyframes writes no start value", (_writer, split) => {
const script = `const tl = gsap.timeline({ paused: true });
tl.to("#a", { opacity: 0.5, duration: 2, keyframes: steps() }, 0);`;
const result = split(script, { ...opts, splitTime: 1 });
expect(result.script).not.toContain('"#a-2"');
expect(result.skippedSelectors).toContain("#a (keyframes spanning split)");
});
it("treats keyframes with a computed percentage key as unreadable", () => {
const script = `const tl = gsap.timeline({ paused: true });
tl.to("#a", { duration: 1, keyframes: { [k]: { x: 10 }, "100%": { x: 50 } } }, 0);`;
const id = acornId(script);
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
expect(updateAnimAcorn(script, id, { properties: { opacity: 0.5 } })).toBe(script);
expect(removeKeyframeAcorn(script, id, 100)).toBe(script);
});
it.each([
["a named step", '{ "0%": { x: 0 }, "100%": last }'],
["a spread inside a step", '{ "0%": { x: 0 }, "100%": { ...base, x: 10 } }'],
["a numeric key", '{ "0%": { x: 0 }, [50]: { x: 5 }, "100%": { x: 10 } }'],
["a template key in an array step", "[{ x: 0 }, { x: 10, [`y`]: 5 }]"],
["a variable key in an array step", "[{ x: 0 }, { x: 10, [k]: 5 }]"],
["a getter in a step", '{ "0%": { x: 0 }, "100%": { get x() { return 10; } } }'],
["a method in an array step", "[{ x: 0 }, { x: 10, y() {} }]"],
["an array channel beside % steps", '{ "0%": { x: 0 }, "100%": { x: 10 }, y: [0, 5] }'],
["an unresolved array value", "{ x: [0, 10, v] }"],
["a spread in an array channel", "{ x: [0, ...rest] }"],
["a hole in an array channel", "{ x: [0, , 10] }"],
])(
"treats keyframes with %s as unreadable, so drags and deletes keep them",
(_shape, keyframes) => {
const script = `const tl = gsap.timeline({ paused: true });
tl.to("#a", { duration: 2, keyframes: ${keyframes} }, 0);`;
const id = acornId(script);
expect(parseGsapScriptAcorn(script).animations[0]!.hasUnresolvedKeyframes).toBe(true);
expect(moveKeyframeAcorn(script, id, 100, 80)).toBe(script);
expect(removeKeyframeAcorn(script, id, 0)).toBe(script);
},
);
it("recast property edit leaves keyframes from a call unchanged", () => {
const script = `const tl = gsap.timeline({ paused: true });
tl.to("#a", { x: 5, duration: 1, keyframes: steps() }, 0);`;
expect(updateAnimRecast(script, acornId(script), { properties: { x: 9 } })).toBe(script);
});
});
// ─── arc path parity ──────────────────────────────────────────────────────────
const ARC_FLAT_SCRIPT = `
+30 -16
View File
@@ -14,6 +14,7 @@ import type {
ArcPathSegment,
} from "./gsapSerialize.js";
import {
authorsKeyframes,
resolveConversionProps,
extractArcWaypoints,
buildMotionPathObjectCode,
@@ -329,7 +330,7 @@ export function updateAnimationInScript(
const parsed = parseGsapScriptAcornForWrite(script);
if (!parsed) return script;
const target = parsed.located.find((l) => l.id === animationId);
if (!target) return script;
if (!target || (updates.properties && target.animation.hasUnresolvedKeyframes)) return script;
const ms = new MagicString(script);
const { call }: { call: TweenCallInfo } = target;
@@ -1078,7 +1079,7 @@ export function updateKeyframeInScript(
const parsed = parseGsapScriptAcornForWrite(script);
if (!parsed) return script;
const target = parsed.located.find((l) => l.id === animationId);
if (!target) return script;
if (!target || target.animation.hasUnresolvedKeyframes) return script;
const kfPropNode = findPropertyNode(target.call.varsArg, "keyframes");
if (!kfPropNode) {
@@ -1198,7 +1199,7 @@ function locateWithKeyframes(
const target =
parsed.located.find((l) => l.id === animationId) ??
parsed.located.find((l) => l.id === convertedId);
if (!target) return null;
if (!target || target.animation.hasUnresolvedKeyframes) return null;
const kfPropNode = findPropertyNode(target.call.varsArg, "keyframes");
if (!kfPropNode || kfPropNode.value?.type !== "ObjectExpression") return null;
return { script, parsed, target, kfNode: kfPropNode.value };
@@ -1246,7 +1247,7 @@ function ensureKeyframesNode(
const parsed = parseGsapScriptAcornForWrite(script);
const target = parsed?.located.find((l) => l.id === animationId);
if (!target) return null;
if (!target || target.animation.hasUnresolvedKeyframes) return null;
// Array-form keyframes → normalize to object form, then re-locate.
const kfProp = findPropertyNode(target.call.varsArg, "keyframes");
@@ -1422,7 +1423,7 @@ export function removeKeyframeFromScript(
const parsed = parseGsapScriptAcornForWrite(script);
if (!parsed) return script;
const target = parsed.located.find((l) => l.id === animationId);
if (!target) return script;
if (!target || target.animation.hasUnresolvedKeyframes) return script;
const kfPropNode = findPropertyNode(target.call.varsArg, "keyframes");
if (!kfPropNode) return script;
@@ -1691,7 +1692,7 @@ export function convertToKeyframesFromScript(
const target = parsed.located.find((l) => l.id === animationId);
if (!target) return script;
const { animation, call } = target;
if (animation.keyframes) return script;
if (authorsKeyframes(animation)) return script;
const isSet = call.method === "set";
const { fromProps, toProps } = resolveConversionProps(animation, resolvedFromValues);
@@ -2028,6 +2029,7 @@ export function splitIntoPropertyGroupsFromScript(
const target = parsed.located.find((l) => l.id === animationId);
if (!target) return { script, ids: [animationId] };
const { animation } = target;
if (animation.hasUnresolvedKeyframes) return { script, ids: [animationId] };
const allPropKeys = collectPropertyKeys(animation);
const groupProps = partitionPropertyGroups(allPropKeys);
@@ -2527,8 +2529,8 @@ function computeForwardBaselines(
const dur = anim.duration ?? 0;
const animEnd = pos + dur;
if (anim.keyframes) {
const kfs = anim.keyframes.keyframes;
if (authorsKeyframes(anim)) {
const kfs = anim.keyframes?.keyframes ?? [];
if (pos >= splitTime) {
// Moves wholly to the new element — contributes nothing to the baseline.
} else if (animEnd > splitTime) {
@@ -2613,6 +2615,24 @@ type SplitCtx = {
newElementStart: number;
};
function applyKeyframedTweenSplit(
result: string,
anim: GsapAnimation,
pos: number,
animEnd: number,
ctx: SplitCtx,
skippedSelectors: string[],
): string {
if (pos >= ctx.splitTime)
return updateAnimationSelectorInScript(result, anim.id, ctx.newSelector);
if (animEnd > ctx.splitTime) {
skippedSelectors.push(`${ctx.originalSelector} (keyframes spanning split)`);
} else if (anim.hasUnresolvedKeyframes) {
skippedSelectors.push(`${ctx.originalSelector} (unreadable keyframes before split)`);
}
return result;
}
// Decide what one matching tween does at the split point: move to the new
// element (wholly after), stay (wholly before / keyframes before), get skipped
// (keyframes spanning), or get interpolated in half (spanning). Returns the
@@ -2628,14 +2648,8 @@ function applyTweenSplit(
const dur = anim.duration ?? 0;
const animEnd = pos + dur;
if (anim.keyframes) {
if (pos >= ctx.splitTime)
return updateAnimationSelectorInScript(result, anim.id, ctx.newSelector);
if (animEnd > ctx.splitTime) {
skippedSelectors.push(`${ctx.originalSelector} (keyframes spanning split)`);
}
// Inherited-state for kf tweens is handled by computeForwardBaselines.
return result;
if (authorsKeyframes(anim)) {
return applyKeyframedTweenSplit(result, anim, pos, animEnd, ctx, skippedSelectors);
}
// Wholly before the split — kept on the original element.
if (animEnd <= ctx.splitTime) return result;