Skip to content

Commit dffeeb5

Browse files
skovhuscursoragent
andauthored
Preserve var() in animation shorthand (#389)
* Preserve var() in animation shorthand The animation shorthand parser was silently dropping var() function tokens because they did not match any of the time/timing/direction/ fill-mode/play-state/iteration classifiers. For example, animation: shimmer var(--animation-duration, 1.5s) infinite would lose the var() and emit only animationName + animationIterationCount. Recognize var() tokens and assign them positionally per CSS shorthand semantics: bind to the longhand hinted by the var()'s fallback when possible, otherwise fall back to the next free time slot (duration first, then delay). Co-authored-by: Kenneth Skovhus <skovhus@users.noreply.github.com> * Bail when animation var() type cannot be inferred Per review: previously, var() tokens with no classifiable fallback (e.g. `var(--anim-token)`) were coerced into a duration/delay slot. This silently miscompiles cases like `${kf} var(--anim-token) 2s infinite` where `--anim-token: ease-in` resolves at runtime — the browser would treat `2s` as the duration. Now bail and emit a warning when an animation shorthand contains a var() whose runtime longhand position cannot be determined statically. The shorthand is left intact (no transformation), preserving original semantics. Users can rebind the variable to a specific longhand (e.g. `animation-duration: var(--x)`) to opt in to transformation. Co-authored-by: Kenneth Skovhus <skovhus@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent e39f77c commit dffeeb5

8 files changed

+334
-16
lines changed

scripts/verify-storybook-rendering.mts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const FLAKINESS_EXPECTED = new Set<string>([
5858
"keyframes-interpolatedDurationWithDelay",
5959
"keyframes-multiAnimationInterpolatedDuration",
6060
"keyframes-unionComplexity",
61+
"cssVariable-animationShorthand",
6162
]);
6263

6364
// Case-specific pixelmatch threshold overrides for known anti-aliasing noise.

src/internal/keyframes.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,9 +488,13 @@ export function expandStaticAnimationShorthand(
488488
const jsName = nameMap?.get(cssName) ?? cssKeyframeNameToIdentifier(cssName);
489489
const remaining = tokens.filter((_, i) => i !== nameIdx);
490490

491+
const classified = classifyAnimationTokens(remaining);
492+
if (!classified) {
493+
return false;
494+
}
495+
491496
styleObj.animationName = j.identifier(jsName);
492497

493-
const classified = classifyAnimationTokens(remaining);
494498
if (classified.duration) {
495499
styleObj.animationDuration = classified.duration;
496500
}

src/internal/logger.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ export type WarningType =
110110
| "Using styled-components components as mixins is not supported; use css`` mixins or strings instead"
111111
| "styled(ImportedComponent) wraps a component whose file contains internal styled-components — convert the base component's file first to avoid CSS cascade conflicts"
112112
| "Transient $-prefixed props renamed on exported component — update consumer call sites to use the new prop names"
113-
| "Shorthand property has an opaque value that StyleX will expand to longhands — use `directional` in resolveValue to return separate longhand tokens";
113+
| "Shorthand property has an opaque value that StyleX will expand to longhands — use `directional` in resolveValue to return separate longhand tokens"
114+
| "animation shorthand contains a var() with no classifiable fallback — its longhand position cannot be determined statically; bind the variable to a specific longhand (e.g. animation-duration: var(--x)) instead";
114115

115116
export interface WarningLog {
116117
severity: Severity;

src/internal/lower-rules/animation.ts

Lines changed: 171 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ const DIRECTIONS = new Set(["normal", "reverse", "alternate", "alternate-reverse
2828
const FILL_MODES = new Set(["none", "forwards", "backwards", "both"]);
2929
const PLAY_STATES = new Set(["running", "paused"]);
3030

31+
/** Matches a `var(...)` CSS function call at the top level of a token. */
32+
const VAR_TOKEN_RE = /^var\(/;
33+
3134
function isTimeToken(t: string): boolean {
3235
return TIME_RE.test(t);
3336
}
@@ -52,8 +55,93 @@ function isIterationCount(t: string): boolean {
5255
return t === "infinite" || /^\d+$/.test(t);
5356
}
5457

58+
function isVarToken(t: string): boolean {
59+
return VAR_TOKEN_RE.test(t);
60+
}
61+
62+
type AnimLonghandCategory =
63+
| "duration"
64+
| "delay"
65+
| "timing"
66+
| "direction"
67+
| "fillMode"
68+
| "playState"
69+
| "iteration";
70+
71+
/** Non-time longhand classifiers, in match priority order. */
72+
const NON_TIME_CLASSIFIERS: ReadonlyArray<{
73+
category: Exclude<AnimLonghandCategory, "duration" | "delay">;
74+
predicate: (t: string) => boolean;
75+
}> = [
76+
{ category: "timing", predicate: isTimingFunction },
77+
{ category: "direction", predicate: isDirection },
78+
{ category: "fillMode", predicate: isFillMode },
79+
{ category: "playState", predicate: isPlayState },
80+
{ category: "iteration", predicate: isIterationCount },
81+
];
82+
83+
/**
84+
* Returns the longhand category that a `var(...)` token's fallback hints at,
85+
* or null if the token has no detectable fallback type. The fallback is the
86+
* portion after the first comma at the top level of the var() call. `"time"`
87+
* indicates the token should be assigned positionally as duration/delay.
88+
*
89+
* E.g. `var(--x, 1.5s)` → `"time"`, `var(--x, ease-in)` → `"timing"`,
90+
* `var(--x)` or `var(--x, somethingUnknown)` → `null`.
91+
*/
92+
function classifyVarTokenFallback(
93+
token: string,
94+
): "time" | Exclude<AnimLonghandCategory, "duration" | "delay"> | null {
95+
const fallback = extractVarFallback(token);
96+
if (!fallback) {
97+
return null;
98+
}
99+
if (isTimeToken(fallback)) {
100+
return "time";
101+
}
102+
for (const { category, predicate } of NON_TIME_CLASSIFIERS) {
103+
if (predicate(fallback)) {
104+
return category;
105+
}
106+
}
107+
return null;
108+
}
109+
110+
/**
111+
* Extracts the fallback value from a `var(--name, fallback)` token.
112+
* Returns the trimmed fallback string, or null if there is no fallback
113+
* or the input is not a well-formed var() call.
114+
*/
115+
function extractVarFallback(token: string): string | null {
116+
if (!isVarToken(token) || !token.endsWith(")")) {
117+
return null;
118+
}
119+
const inner = token.slice(4, -1);
120+
let depth = 0;
121+
for (let i = 0; i < inner.length; i++) {
122+
const ch = inner[i];
123+
if (ch === "(") {
124+
depth++;
125+
} else if (ch === ")") {
126+
depth--;
127+
} else if (ch === "," && depth === 0) {
128+
return inner.slice(i + 1).trim();
129+
}
130+
}
131+
return null;
132+
}
133+
55134
/**
56135
* Classifies animation tokens (excluding the animation name) into longhand categories.
136+
*
137+
* `var(...)` tokens are bound to the longhand hinted by their fallback value
138+
* (e.g., a time literal → duration/delay, an easing keyword → timing-function).
139+
* When a var()'s fallback cannot be classified (e.g., `var(--x)` with no
140+
* fallback, or `var(--x, foo)` with an unrecognized fallback), the runtime
141+
* value is unknown and could resolve to any longhand category — coercing it
142+
* to a time slot would change semantics for valid shorthands like
143+
* `${kf} var(--token) 2s infinite` where `--token: ease-in`. In that case
144+
* this function returns `null` to signal that the caller should bail.
57145
*/
58146
export function classifyAnimationTokens(tokens: string[]): {
59147
duration: string | null;
@@ -63,17 +151,62 @@ export function classifyAnimationTokens(tokens: string[]): {
63151
fillMode: string | null;
64152
playState: string | null;
65153
iteration: string | null;
66-
} {
67-
const timeTokens = tokens.filter(isTimeToken);
68-
return {
69-
duration: timeTokens[0] ?? null,
70-
delay: timeTokens[1] ?? null,
71-
timing: tokens.find(isTimingFunction) ?? null,
72-
direction: tokens.find(isDirection) ?? null,
73-
fillMode: tokens.find(isFillMode) ?? null,
74-
playState: tokens.find(isPlayState) ?? null,
75-
iteration: tokens.find(isIterationCount) ?? null,
154+
} | null {
155+
const result: Record<AnimLonghandCategory, string | null> = {
156+
duration: null,
157+
delay: null,
158+
timing: null,
159+
direction: null,
160+
fillMode: null,
161+
playState: null,
162+
iteration: null,
76163
};
164+
165+
const assignTime = (t: string): boolean => {
166+
if (result.duration === null) {
167+
result.duration = t;
168+
return true;
169+
}
170+
if (result.delay === null) {
171+
result.delay = t;
172+
return true;
173+
}
174+
return false;
175+
};
176+
177+
for (const t of tokens) {
178+
if (isVarToken(t)) {
179+
const hinted = classifyVarTokenFallback(t);
180+
if (hinted === null) {
181+
// Unknown var() type — runtime value could be anything. Bail.
182+
return null;
183+
}
184+
if (hinted === "time") {
185+
if (!assignTime(t)) {
186+
return null;
187+
}
188+
continue;
189+
}
190+
if (result[hinted] !== null) {
191+
// Conflicting var() with same hinted longhand already assigned. Bail.
192+
return null;
193+
}
194+
result[hinted] = t;
195+
continue;
196+
}
197+
if (isTimeToken(t)) {
198+
assignTime(t);
199+
continue;
200+
}
201+
for (const { category, predicate } of NON_TIME_CLASSIFIERS) {
202+
if (result[category] === null && predicate(t)) {
203+
result[category] = t;
204+
break;
205+
}
206+
}
207+
}
208+
209+
return result;
77210
}
78211

79212
export function tryHandleAnimation(args: {
@@ -91,6 +224,7 @@ export function tryHandleAnimation(args: {
91224
value: unknown,
92225
commentSource: { leading?: string; trailingLine?: string } | null,
93226
) => void;
227+
bailUnsupportedUnknownVar?: () => void;
94228
}): boolean {
95229
const { j, decl, d, keyframesNames, styleObj, styleFnDecls, styleFnFromProps } = args;
96230
const applyProp =
@@ -232,7 +366,11 @@ export function tryHandleAnimation(args: {
232366
| { kind: "interpolated"; slotId: number; unit: string; originalIndex: number };
233367
const timeSlots: TimeSlot[] = [];
234368

235-
// First pass: collect all time tokens with original indices
369+
// First pass: collect all time tokens with original indices.
370+
// `var(...)` tokens are bound to time slots only when their fallback is
371+
// a time literal; var() with non-time fallbacks is left for the longhand
372+
// classifier; var() with no classifiable fallback bails (we cannot know
373+
// its runtime category — see classifyAnimationTokens for details).
236374
for (let i = 0; i < tokens.length; i++) {
237375
const token = tokens[i]!;
238376
const interpMatch = token.match(INTERPOLATED_TIME_RE);
@@ -243,23 +381,42 @@ export function tryHandleAnimation(args: {
243381
unit: interpMatch[2]!,
244382
originalIndex: i,
245383
});
246-
} else if (isTimeToken(token)) {
384+
continue;
385+
}
386+
if (isTimeToken(token)) {
247387
timeSlots.push({ kind: "static", value: token, originalIndex: i });
388+
continue;
389+
}
390+
if (isVarToken(token)) {
391+
const hinted = classifyVarTokenFallback(token);
392+
if (hinted === null) {
393+
args.bailUnsupportedUnknownVar?.();
394+
return false;
395+
}
396+
if (hinted === "time") {
397+
timeSlots.push({ kind: "static", value: token, originalIndex: i });
398+
}
248399
}
249400
}
250401

251402
// Sort by original index to preserve CSS shorthand order
252403
timeSlots.sort((a, b) => a.originalIndex - b.originalIndex);
253404

254-
// Remove interpolated time tokens from the array for classifyAnimationTokens
405+
// Remove tokens already assigned to time slots so the longhand
406+
// classifier does not double-count them.
407+
const consumedIndices = new Set(timeSlots.map((s) => s.originalIndex));
255408
for (let i = tokens.length - 1; i >= 0; i--) {
256-
if (INTERPOLATED_TIME_RE.test(tokens[i]!)) {
409+
if (consumedIndices.has(i) || INTERPOLATED_TIME_RE.test(tokens[i]!)) {
257410
tokens.splice(i, 1);
258411
}
259412
}
260413

261414
// Classify remaining (non-interpolated) tokens into animation longhand categories
262415
const classified = classifyAnimationTokens(tokens);
416+
if (!classified) {
417+
args.bailUnsupportedUnknownVar?.();
418+
return false;
419+
}
263420

264421
// Reset duration/delay from classified — we'll reassign based on original order.
265422
let durationValue: AnimLonghandValue = null;

src/internal/lower-rules/rule-interpolated-declaration.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,10 +345,18 @@ export function handleInterpolatedDeclaration(args: InterpolatedDeclarationConte
345345
filePath,
346346
avoidNames,
347347
applyResolvedPropValue,
348+
bailUnsupportedUnknownVar: () =>
349+
bailUnsupportedLocal(
350+
decl,
351+
"animation shorthand contains a var() with no classifiable fallback — its longhand position cannot be determined statically; bind the variable to a specific longhand (e.g. animation-duration: var(--x)) instead",
352+
),
348353
})
349354
) {
350355
continue;
351356
}
357+
if (bail) {
358+
break;
359+
}
352360
if (isPseudoElementSelector(pseudoElement)) {
353361
if (tryHandleDynamicPseudoElementStyleFunction(args)) {
354362
continue;
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// @expected-warning: animation shorthand contains a var() with no classifiable fallback — its longhand position cannot be determined statically; bind the variable to a specific longhand (e.g. animation-duration: var(--x)) instead
2+
// var() inside an animation shorthand with no classifiable fallback type is
3+
// unsupported because we cannot statically determine which longhand position
4+
// it should map to (could be duration, timing-function, etc. at runtime).
5+
// Coercing it would miscompile cases like `--anim-token: ease-in` where the
6+
// browser would treat the following `2s` as the duration.
7+
import styled, { keyframes } from "styled-components";
8+
9+
const pulse = keyframes`
10+
0%, 100% { transform: scale(1); }
11+
50% { transform: scale(1.1); }
12+
`;
13+
14+
const Ambiguous = styled.div`
15+
width: 40px;
16+
height: 40px;
17+
background-color: lavender;
18+
animation: ${pulse} var(--anim-token) 2s infinite;
19+
`;
20+
21+
export const App = () => <Ambiguous>Amb</Ambiguous>;
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// CSS var() inside animation shorthand should be preserved as animationDuration
2+
// (or another longhand inferred from the var()'s fallback type), not silently dropped.
3+
import styled, { keyframes } from "styled-components";
4+
5+
const shimmer = keyframes`
6+
0% { opacity: 0.4; }
7+
50% { opacity: 1; }
8+
100% { opacity: 0.4; }
9+
`;
10+
11+
const pulse = keyframes`
12+
0%, 100% { transform: scale(1); }
13+
50% { transform: scale(1.1); }
14+
`;
15+
16+
// var() with a time fallback → animationDuration
17+
const ProgressFill = styled.div`
18+
position: relative;
19+
height: 8px;
20+
background-color: cornflowerblue;
21+
&::after {
22+
content: "";
23+
position: absolute;
24+
inset: 0;
25+
opacity: var(--animation-enabled, 0);
26+
animation: ${shimmer} var(--animation-duration, 1.5s) infinite;
27+
animation-timing-function: ease-in-out;
28+
}
29+
`;
30+
31+
// var() with a timing-function fallback → animationTimingFunction
32+
const Pulser = styled.div`
33+
width: 40px;
34+
height: 40px;
35+
background-color: tomato;
36+
animation: ${pulse} 2s var(--easing, ease-in-out) infinite;
37+
`;
38+
39+
// Two var() time values → duration then delay
40+
const Delayed = styled.div`
41+
width: 40px;
42+
height: 40px;
43+
background-color: gold;
44+
animation: ${pulse} var(--dur, 0.8s) var(--delay, 0.2s) ease-out infinite;
45+
`;
46+
47+
export const App = () => (
48+
<div style={{ display: "flex", gap: 16, padding: 16 }}>
49+
<ProgressFill>Progress</ProgressFill>
50+
<Pulser>Pulse</Pulser>
51+
<Delayed>Delay</Delayed>
52+
</div>
53+
);

0 commit comments

Comments
 (0)