Skip to content

Commit 89a264e

Browse files
authored
Fix keyframes template interpolation and animation longhand splitting (#376)
* Fix keyframes template interpolation and animation longhand splitting * Support named styled import in collection and preflight * Bail keyframes conversion for non-static slot expressions * Fix keyframes static guard type narrowing
1 parent 135b8de commit 89a264e

File tree

8 files changed

+429
-35
lines changed

8 files changed

+429
-35
lines changed

src/__tests__/transform.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5459,6 +5459,36 @@ export const App = () => <Box $animate>Multi</Box>;
54595459
});
54605460
});
54615461

5462+
describe("keyframes interpolation safety", () => {
5463+
it("should not convert keyframes when a slot expression resolves to a non-static binding", () => {
5464+
const source = `
5465+
import styled, { keyframes } from "styled-components";
5466+
5467+
const dynamicOpacity = () => 0.5;
5468+
5469+
const fade = keyframes\`
5470+
from {
5471+
opacity: \${dynamicOpacity};
5472+
}
5473+
to {
5474+
opacity: 1;
5475+
}
5476+
\`;
5477+
5478+
const Box = styled.div\`
5479+
animation: \${fade} 1s linear;
5480+
\`;
5481+
5482+
export const App = () => <Box />;
5483+
`;
5484+
5485+
const result = runTransformWithDiagnostics(source);
5486+
expect(result.code).not.toBeNull();
5487+
expect(result.code).toContain("const fade = keyframes`");
5488+
expect(result.code).not.toContain("const fade = stylex.keyframes(");
5489+
});
5490+
});
5491+
54625492
describe("binary expression with bare props param usage", () => {
54635493
it("should bail when the arrow function body passes props as a bare argument", () => {
54645494
const source = `

src/internal/keyframes.ts

Lines changed: 219 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
* Converts styled-components keyframes into StyleX keyframes objects.
33
* Core concepts: Stylis parsing and keyframes extraction.
44
*/
5-
import type { ASTNode, Collection, ImportDeclaration, JSCodeshift } from "jscodeshift";
5+
import type { ASTNode, ASTPath, Collection, ImportDeclaration, JSCodeshift } from "jscodeshift";
66
import valueParser from "postcss-value-parser";
77
import { compile } from "stylis";
88
import type { CssRuleIR } from "./css-ir.js";
99
import { cssDeclarationToStylexDeclarations } from "./css-prop-mapping.js";
1010
import { classifyAnimationTokens } from "./lower-rules/animation.js";
11+
import { cloneAstNode, literalToStaticValue } from "./utilities/jscodeshift-utils.js";
1112

1213
export function convertStyledKeyframes(args: {
1314
root: Collection<ASTNode>;
@@ -21,15 +22,30 @@ export function convertStyledKeyframes(args: {
2122

2223
function parseKeyframesTemplate(args: {
2324
template: ASTNode | null | undefined;
25+
j: JSCodeshift;
26+
scopePath: ASTPath<ASTNode>;
2427
}): Record<string, Record<string, unknown>> | null {
25-
const { template } = args;
28+
const { template, j, scopePath } = args;
2629
if (!template || template.type !== "TemplateLiteral") {
2730
return null;
2831
}
29-
if ((template.expressions?.length ?? 0) > 0) {
30-
return null;
32+
const slotExprById = new Map<number, ExpressionKind>();
33+
for (let i = 0; i < (template.expressions?.length ?? 0); i++) {
34+
const expr = template.expressions[i];
35+
if (!expr) {
36+
return null;
37+
}
38+
if (!isStaticSafeKeyframesSlotExpression(expr as ExpressionKind, scopePath)) {
39+
return null;
40+
}
41+
slotExprById.set(i, expr as ExpressionKind);
3142
}
32-
const rawCss = (template.quasis ?? []).map((q: any) => q.value?.raw ?? "").join("");
43+
const rawCss = (template.quasis ?? [])
44+
.map((q: any, i: number) => {
45+
const raw = q.value?.raw ?? "";
46+
return i < (template.expressions?.length ?? 0) ? `${raw}__SC_EXPR_${i}__` : raw;
47+
})
48+
.join("");
3349
const wrapped = `@keyframes __SC_KEYFRAMES__ { ${rawCss} }`;
3450
const ast = compile(wrapped) as any[];
3551

@@ -81,7 +97,7 @@ function parseKeyframesTemplate(args: {
8197
continue;
8298
}
8399
const raw = propRaw.trim();
84-
applyStaticDeclsToStyleObj(styleObj, raw, valueRaw);
100+
applyStaticDeclsToStyleObj(styleObj, raw, valueRaw, { j, slotExprById });
85101
}
86102

87103
frames[frameKey] = styleObj;
@@ -124,7 +140,7 @@ function convertStyledKeyframesImpl(args: {
124140
}
125141
const localName = p.node.id.name;
126142
const template = init?.quasi;
127-
const frames = parseKeyframesTemplate({ template });
143+
const frames = parseKeyframesTemplate({ template, j, scopePath: p as ASTPath<ASTNode> });
128144
if (!frames) {
129145
return;
130146
}
@@ -229,6 +245,10 @@ function applyStaticDeclsToStyleObj(
229245
styleObj: Record<string, unknown>,
230246
property: string,
231247
valueRaw: string,
248+
options?: {
249+
j?: JSCodeshift;
250+
slotExprById?: Map<number, ExpressionKind>;
251+
},
232252
): void {
233253
for (const out of cssDeclarationToStylexDeclarations({
234254
property,
@@ -238,11 +258,203 @@ function applyStaticDeclsToStyleObj(
238258
})) {
239259
if (out.value.kind === "static") {
240260
const v = out.value.value.trim();
261+
const exprValue = resolvePlaceholderValueToAst(v, options);
262+
if (exprValue) {
263+
styleObj[out.prop] = exprValue;
264+
continue;
265+
}
241266
styleObj[out.prop] = /^-?\d*\.?\d+$/.test(v) ? Number(v) : v;
242267
}
243268
}
244269
}
245270

271+
function resolvePlaceholderValueToAst(
272+
value: string,
273+
options:
274+
| {
275+
j?: JSCodeshift;
276+
slotExprById?: Map<number, ExpressionKind>;
277+
}
278+
| null
279+
| undefined,
280+
): ExpressionKind | null {
281+
const j = options?.j;
282+
const slotExprById = options?.slotExprById;
283+
if (!j || !slotExprById || !/__SC_EXPR_\d+__/.test(value)) {
284+
return null;
285+
}
286+
287+
const placeholderRe = /__SC_EXPR_(\d+)__/g;
288+
const quasis: any[] = [];
289+
const exprs: ExpressionKind[] = [];
290+
let lastIndex = 0;
291+
let match: RegExpExecArray | null;
292+
293+
while ((match = placeholderRe.exec(value))) {
294+
const expr = slotExprById.get(Number(match[1]));
295+
if (!expr) {
296+
return null;
297+
}
298+
const prefix = value.slice(lastIndex, match.index);
299+
quasis.push(j.templateElement({ raw: prefix, cooked: prefix }, false));
300+
exprs.push(cloneAstNode(expr));
301+
lastIndex = match.index + match[0].length;
302+
}
303+
304+
if (exprs.length === 0) {
305+
return null;
306+
}
307+
308+
const suffix = value.slice(lastIndex);
309+
quasis.push(j.templateElement({ raw: suffix, cooked: suffix }, true));
310+
311+
if (quasis.length === 2 && quasis[0].value.raw === "" && quasis[1].value.raw === "") {
312+
return exprs[0]!;
313+
}
314+
315+
return j.templateLiteral(quasis, exprs);
316+
}
317+
318+
function isStaticSafeKeyframesSlotExpression(
319+
expr: ExpressionKind,
320+
scopePath: ASTPath<ASTNode>,
321+
seenIdentifiers: Set<string> = new Set(),
322+
): boolean {
323+
if (isFunctionLikeExpression(expr)) {
324+
return false;
325+
}
326+
327+
const staticValue = literalToStaticValue(expr);
328+
if (staticValue !== null) {
329+
return typeof staticValue === "string" || typeof staticValue === "number";
330+
}
331+
332+
if (expr.type === "Identifier") {
333+
return isStaticSafeIdentifierBinding(expr.name, scopePath, seenIdentifiers);
334+
}
335+
336+
if (expr.type === "UnaryExpression") {
337+
return isStaticSafeKeyframesSlotExpression(
338+
expr.argument as ExpressionKind,
339+
scopePath,
340+
seenIdentifiers,
341+
);
342+
}
343+
344+
if (expr.type === "BinaryExpression" || expr.type === "LogicalExpression") {
345+
return (
346+
isStaticSafeKeyframesSlotExpression(
347+
expr.left as ExpressionKind,
348+
scopePath,
349+
seenIdentifiers,
350+
) &&
351+
isStaticSafeKeyframesSlotExpression(expr.right as ExpressionKind, scopePath, seenIdentifiers)
352+
);
353+
}
354+
355+
if (expr.type === "ConditionalExpression") {
356+
return (
357+
isStaticSafeKeyframesSlotExpression(
358+
expr.test as ExpressionKind,
359+
scopePath,
360+
seenIdentifiers,
361+
) &&
362+
isStaticSafeKeyframesSlotExpression(
363+
expr.consequent as ExpressionKind,
364+
scopePath,
365+
seenIdentifiers,
366+
) &&
367+
isStaticSafeKeyframesSlotExpression(
368+
expr.alternate as ExpressionKind,
369+
scopePath,
370+
seenIdentifiers,
371+
)
372+
);
373+
}
374+
375+
if (expr.type === "TemplateLiteral") {
376+
return expr.expressions.every((slotExpr) =>
377+
isStaticSafeKeyframesSlotExpression(slotExpr as ExpressionKind, scopePath, seenIdentifiers),
378+
);
379+
}
380+
381+
if (expr.type === "ParenthesizedExpression") {
382+
return isStaticSafeKeyframesSlotExpression(
383+
expr.expression as ExpressionKind,
384+
scopePath,
385+
seenIdentifiers,
386+
);
387+
}
388+
389+
if (
390+
expr.type === "TSAsExpression" ||
391+
expr.type === "TSTypeAssertion" ||
392+
expr.type === "TSSatisfiesExpression"
393+
) {
394+
return isStaticSafeKeyframesSlotExpression(
395+
expr.expression as ExpressionKind,
396+
scopePath,
397+
seenIdentifiers,
398+
);
399+
}
400+
401+
return false;
402+
}
403+
404+
function isFunctionLikeExpression(expr: ExpressionKind): boolean {
405+
return expr.type === "ArrowFunctionExpression" || expr.type === "FunctionExpression";
406+
}
407+
408+
function isStaticSafeIdentifierBinding(
409+
name: string,
410+
scopePath: ASTPath<ASTNode>,
411+
seenIdentifiers: Set<string>,
412+
): boolean {
413+
if (seenIdentifiers.has(name)) {
414+
return false;
415+
}
416+
seenIdentifiers.add(name);
417+
418+
const scope = (scopePath as any).scope?.lookup?.(name);
419+
if (!scope || typeof scope.getBindings !== "function") {
420+
seenIdentifiers.delete(name);
421+
return false;
422+
}
423+
424+
const bindings = scope.getBindings();
425+
const refs = bindings?.[name];
426+
if (!Array.isArray(refs) || refs.length !== 1) {
427+
seenIdentifiers.delete(name);
428+
return false;
429+
}
430+
431+
const idPath = refs[0];
432+
const declarator = idPath?.parent?.value;
433+
if (!declarator || declarator.type !== "VariableDeclarator" || declarator.id !== idPath.value) {
434+
seenIdentifiers.delete(name);
435+
return false;
436+
}
437+
438+
const declaration = idPath?.parent?.parent?.value;
439+
if (!declaration || declaration.type !== "VariableDeclaration" || declaration.kind !== "const") {
440+
seenIdentifiers.delete(name);
441+
return false;
442+
}
443+
444+
if (!declarator.init) {
445+
seenIdentifiers.delete(name);
446+
return false;
447+
}
448+
449+
const isStatic = isStaticSafeKeyframesSlotExpression(
450+
declarator.init as ExpressionKind,
451+
scopePath,
452+
seenIdentifiers,
453+
);
454+
seenIdentifiers.delete(name);
455+
return isStatic;
456+
}
457+
246458
/**
247459
* Expands a static `animation` shorthand value into longhand properties,
248460
* replacing the animation name with a keyframes identifier when it matches

0 commit comments

Comments
 (0)