Skip to content

Commit 688beed

Browse files
cursoragentskovhus
andcommitted
Fix animation: group interpolated times by (longhand, jsxProp) to avoid duplicate properties
When multiple animation segments share the same CSS longhand and component prop (e.g. both fadeIn and slideIn use $duration for animationDuration), the codemod now groups them and emits a single style function with a combined callArg that interpolates all segments. Previously each segment was handled independently, producing duplicate animationDuration properties and hardcoding the non-current segment's value instead of interpolating it. Co-authored-by: Kenneth Skovhus <skovhus@users.noreply.github.com>
1 parent 7d4c456 commit 688beed

File tree

1 file changed

+61
-31
lines changed

1 file changed

+61
-31
lines changed

src/internal/lower-rules/animation.ts

Lines changed: 61 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,9 @@ function computeInterpolatedTimeFallback(
434434
*
435435
* For multi-animation shorthands, the callArg preserves the full comma-separated list
436436
* of durations/delays, with the interpolated segment being dynamic.
437+
*
438+
* When multiple segments share the same longhand and jsxProp, they are grouped into
439+
* a single style function with a combined callArg that interpolates all segments.
437440
*/
438441
function emitInterpolatedAnimTimeFunctions(
439442
j: any,
@@ -445,6 +448,13 @@ function emitInterpolatedAnimTimeFunctions(
445448
delays: Array<string | null>,
446449
avoidNames?: Set<string>,
447450
): void {
451+
// Pre-validate and collect metadata for each interpolated time entry
452+
const validEntries: Array<{
453+
interp: InterpolatedAnimTime;
454+
jsxProp: string;
455+
unwrappedExpr: any;
456+
}> = [];
457+
448458
for (const interp of interpolatedTimes) {
449459
const expr = (decl as any).templateExpressions[interp.slotId] as any;
450460
if (!expr || expr.type !== "ArrowFunctionExpression") {
@@ -463,32 +473,48 @@ function emitInterpolatedAnimTimeFunctions(
463473
continue;
464474
}
465475

466-
// Build the call argument: for multi-animation, include the full comma-separated list
467-
const valuesList = interp.longhand === "animationDuration" ? durations : delays;
468-
const defaultFallback = interp.longhand === "animationDuration" ? "0s" : "0s";
469-
const callArg = buildMultiAnimationCallArg(
470-
j,
471-
unwrapped.expr,
472-
interp.unit,
473-
interp.segmentIndex,
474-
valuesList,
475-
defaultFallback,
476-
);
476+
const jsxProp = [...propsUsed][0];
477+
if (jsxProp) {
478+
validEntries.push({ interp, jsxProp, unwrappedExpr: unwrapped.expr });
479+
}
480+
}
481+
482+
// Group entries by (longhand, jsxProp) so that segments sharing the same
483+
// CSS property and component prop produce a single style function.
484+
const groups = new Map<string, typeof validEntries>();
485+
for (const entry of validEntries) {
486+
const groupKey = `${entry.interp.longhand}:${entry.jsxProp}`;
487+
let group = groups.get(groupKey);
488+
if (!group) {
489+
group = [];
490+
groups.set(groupKey, group);
491+
}
492+
group.push(entry);
493+
}
477494

478-
const cssPropId = cssPropertyToIdentifier(interp.longhand, avoidNames);
479-
const fnKey = styleKeyWithSuffix(decl.styleKey, interp.longhand);
495+
for (const group of groups.values()) {
496+
const { longhand } = group[0]!.interp;
497+
const { jsxProp } = group[0]!;
498+
const valuesList = longhand === "animationDuration" ? durations : delays;
499+
500+
const interpSegments = group.map((entry) => ({
501+
expr: entry.unwrappedExpr,
502+
unit: entry.interp.unit,
503+
segmentIndex: entry.interp.segmentIndex,
504+
}));
505+
506+
const callArg = buildMultiAnimationCallArg(j, interpSegments, valuesList, "0s");
507+
508+
const cssPropId = cssPropertyToIdentifier(longhand, avoidNames);
509+
const fnKey = styleKeyWithSuffix(decl.styleKey, longhand);
480510
if (!styleFnDecls.has(fnKey)) {
481511
const param = j.identifier(cssPropId);
482512
(param as any).typeAnnotation = j.tsTypeAnnotation(j.tsStringKeyword());
483-
const body = j.objectExpression([makeCssProperty(j, interp.longhand, cssPropId)]);
513+
const body = j.objectExpression([makeCssProperty(j, longhand, cssPropId)]);
484514
styleFnDecls.set(fnKey, j.arrowFunctionExpression([param], body));
485515
}
486516

487-
const jsxProp = [...propsUsed][0];
488-
if (jsxProp) {
489-
styleFnFromProps.push({ fnKey, jsxProp, callArg });
490-
}
491-
517+
styleFnFromProps.push({ fnKey, jsxProp, callArg });
492518
decl.needsWrapperComponent = true;
493519
}
494520
}
@@ -497,19 +523,24 @@ function emitInterpolatedAnimTimeFunctions(
497523
* Builds a call argument for multi-animation shorthands.
498524
* For single animation, returns a simple template like `${$duration ?? 200}ms`.
499525
* For multi-animation, returns a template like `${$duration ?? 200}ms, 1s`
500-
* that preserves all animation durations/delays.
526+
* that preserves all animation durations/delays, interpolating all dynamic segments.
501527
*/
502528
function buildMultiAnimationCallArg(
503529
j: any,
504-
interpExpr: any,
505-
unit: string,
506-
segmentIndex: number,
530+
interpSegments: Array<{ expr: any; unit: string; segmentIndex: number }>,
507531
valuesList: Array<string | null>,
508532
defaultFallback: string,
509533
): any {
510-
// Single animation: use simple template
511-
if (valuesList.length === 1) {
512-
return buildTemplateWithStaticParts(j, interpExpr, "", unit);
534+
// Single animation with single interpolation: use simple template
535+
if (valuesList.length === 1 && interpSegments.length === 1) {
536+
const seg = interpSegments[0]!;
537+
return buildTemplateWithStaticParts(j, seg.expr, "", seg.unit);
538+
}
539+
540+
// Build a map from segment index to its interpolated expression
541+
const interpBySegment = new Map<number, { expr: any; unit: string }>();
542+
for (const seg of interpSegments) {
543+
interpBySegment.set(seg.segmentIndex, { expr: seg.expr, unit: seg.unit });
513544
}
514545

515546
// Multi-animation: build template with full list
@@ -522,13 +553,12 @@ function buildMultiAnimationCallArg(
522553
prefix += ", ";
523554
}
524555

525-
if (i === segmentIndex) {
526-
// This is the interpolated segment
556+
const interp = interpBySegment.get(i);
557+
if (interp) {
527558
quasis.push(j.templateElement({ raw: prefix, cooked: prefix }, false));
528-
exprs.push(interpExpr);
529-
prefix = unit;
559+
exprs.push(interp.expr);
560+
prefix = interp.unit;
530561
} else {
531-
// Static segment
532562
prefix += valuesList[i] ?? defaultFallback;
533563
}
534564
}

0 commit comments

Comments
 (0)