Skip to content

Commit 1ac48ad

Browse files
skovhusclaude
andauthored
Support dynamic indexed values in pseudo-element selectors (#353)
* feat: support indexed theme lookup in pseudo-element selectors Extend tryHandleThemeIndexedLookup in value-patterns.ts to handle pseudo-element contexts (::placeholder, ::after, ::before). Previously, indexed theme lookups like `props.theme.color[props.$placeholderColor]` inside pseudo-element selectors were explicitly skipped, producing a bail warning. Now the style function body is wrapped in pseudo-element nesting (e.g., `"::placeholder": { color: $colors[param] }`). Promotes _unimplemented.selector-dynamicPlaceholderIndexed to a supported test case and extends selector-dynamicPseudoElement with an indexed theme variant. https://claude.ai/code/session_019qihykXE8bykukYa1Zefe3 * fix: bail on indexed theme lookup when static prefix/suffix exists Prevents silent data loss when an interpolated value has surrounding static text (e.g., `box-shadow: 0 0 4px ${indexed_theme}`). The indexed expression cannot be concatenated with a prefix in StyleX, so we bail and let downstream handlers deal with it. Addresses PR #353 review feedback. https://claude.ai/code/session_019qihykXE8bykukYa1Zefe3 * fix: guard indexed theme lookups against CSS shorthand properties Adds isCssShorthandProperty() check to tryHandleThemeIndexedLookup so it bails on properties like padding, margin, border, background. These shorthands cannot be emitted directly in StyleX and the indexed theme expression produces a single value that cannot be expanded to longhands. Addresses PR #353 review feedback about shorthand safety. https://claude.ai/code/session_019qihykXE8bykukYa1Zefe3 * refactor: merge indexed theme lookups into pseudo-element style functions Move indexed theme resolution into tryHandleDynamicPseudoElementStyleFunction so that static and dynamic properties produce a single merged StyleX function (e.g., `styles.input({ placeholderColor })`) instead of separate entries (`[styles.input, styles.inputPlaceholderColor(placeholderColor)]`). - Extend isPseudoElementSelector to include ::placeholder - Add tryResolveIndexedThemeForPseudoElement helper for theme resolution - Revert pseudo-element handling from tryHandleThemeIndexedLookup - Fix selector-dynamicPseudoElement test case CSS that blocked playground interaction (replaced absolute-positioned overlays with block-level elements) https://claude.ai/code/session_019qihykXE8bykukYa1Zefe3 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 568eb47 commit 1ac48ad

8 files changed

+403
-73
lines changed

src/__tests__/transform.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5210,3 +5210,51 @@ export const App = () => <Box $depth={2}>Content</Box>;
52105210
expect(result.code).toContain("props.depth * 16 + 4");
52115211
});
52125212
});
5213+
5214+
describe("indexed theme lookup shorthand safety", () => {
5215+
it("should not emit CSS shorthand properties in indexed theme style functions", () => {
5216+
const source = `
5217+
import styled from "styled-components";
5218+
5219+
type Spacing = "sm" | "md";
5220+
5221+
const Pill = styled.span<{ $spacing: Spacing }>\`
5222+
display: inline-block;
5223+
5224+
&::after {
5225+
content: "";
5226+
display: block;
5227+
padding: \${(props) => props.theme.spacing[props.$spacing]};
5228+
}
5229+
\`;
5230+
5231+
export const App = () => <Pill $spacing="sm">Content</Pill>;
5232+
`;
5233+
const result = runTransformWithDiagnostics(source);
5234+
// The codemod should NOT emit "padding" as a shorthand in the style function.
5235+
// It should either expand to longhands or bail on the indexed theme path.
5236+
if (result.code) {
5237+
expect(result.code).not.toMatch(/\bpadding:/);
5238+
}
5239+
});
5240+
5241+
it("should not emit CSS shorthand properties in indexed theme style functions without pseudo-element", () => {
5242+
const source = `
5243+
import styled from "styled-components";
5244+
5245+
type Spacing = "sm" | "md";
5246+
5247+
const Box = styled.div<{ $spacing: Spacing }>\`
5248+
padding: \${(props) => props.theme.spacing[props.$spacing]};
5249+
\`;
5250+
5251+
export const App = () => <Box $spacing="sm">Content</Box>;
5252+
`;
5253+
const result = runTransformWithDiagnostics(source);
5254+
// The codemod should NOT emit "padding" as a shorthand in the style function.
5255+
// Bailing (code === null) is also acceptable — better than emitting invalid StyleX.
5256+
if (result.code) {
5257+
expect(result.code).not.toMatch(/\bpadding:/);
5258+
}
5259+
});
5260+
});

src/internal/css-prop-mapping.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import type { CssDeclarationIR, CssValue, CssValuePart } from "./css-ir.js";
66
import { splitDirectionalProperty } from "./stylex-shorthands.js";
77
import { isBackgroundImageValue, looksLikeLength } from "./utilities/string-utils.js";
88

9+
export { isCssShorthandProperty };
10+
911
type StylexPropDecl = { prop: string; value: CssValue };
1012

1113
type DirectionalProp = "padding" | "margin" | "scrollMargin" | "scrollPadding";
@@ -17,6 +19,19 @@ const DIRECTIONAL_SHORTHAND_MAP: Record<string, DirectionalProp> = {
1719
"scroll-padding": "scrollPadding",
1820
};
1921

22+
/**
23+
* Returns true if the CSS property is a shorthand that StyleX cannot express directly
24+
* and requires expansion (e.g., `padding`, `margin`, `border`, `background`).
25+
*/
26+
function isCssShorthandProperty(cssProp: string): boolean {
27+
return (
28+
cssProp in DIRECTIONAL_SHORTHAND_MAP ||
29+
cssProp === "border" ||
30+
/^border-(top|right|bottom|left)$/.test(cssProp) ||
31+
cssProp === "background"
32+
);
33+
}
34+
2035
/**
2136
* For a `background` CSS property, determine the appropriate StyleX property name.
2237
* Returns `backgroundImage` for gradients/images, `backgroundColor` for colors.

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

Lines changed: 167 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { resolveDynamicNode } from "../builtin-handlers.js";
1414
import {
1515
cssDeclarationToStylexDeclarations,
1616
cssPropertyToStylexProp,
17+
isCssShorthandProperty,
1718
parseBorderShorthandParts,
1819
resolveBackgroundStylexProp,
1920
} from "../css-prop-mapping.js";
@@ -63,6 +64,7 @@ import { extractUnionLiteralValues } from "./variants.js";
6364
import { toStyleKey, styleKeyWithSuffix } from "../transform/helpers.js";
6465
import { cssPropertyToIdentifier, makeCssProperty, makeCssPropKey } from "./shared.js";
6566
import { isMemberExpression } from "./utils.js";
67+
import { extractIndexedThemeLookupInfo } from "../builtin-handlers/resolver-utils.js";
6668
type CommentSource = { leading?: string; trailingLine?: string } | null;
6769

6870
type InterpolatedDeclarationContext = {
@@ -2283,13 +2285,105 @@ function resolveDerivedLocalVariable(
22832285
}
22842286

22852287
function isPseudoElementSelector(pseudoElement: string | null): boolean {
2286-
return pseudoElement === "::before" || pseudoElement === "::after";
2288+
return (
2289+
pseudoElement === "::before" || pseudoElement === "::after" || pseudoElement === "::placeholder"
2290+
);
2291+
}
2292+
2293+
/**
2294+
* Attempts to resolve an indexed theme lookup from an arrow function expression.
2295+
* Pattern: `(props) => props.theme.color[props.$placeholderColor]`
2296+
* Returns the resolved value expression and metadata, or null if not applicable.
2297+
*/
2298+
function tryResolveIndexedThemeForPseudoElement(
2299+
expr: { type?: string },
2300+
state: DeclProcessingState["state"],
2301+
): {
2302+
valueExpr: ExpressionKind;
2303+
indexPropName: string;
2304+
paramName: string;
2305+
} | null {
2306+
const { resolveValue, resolverImports, parseExpr, api } = state;
2307+
const arrowExpr = expr as {
2308+
type?: string;
2309+
params?: Array<{ type?: string; name?: string }>;
2310+
body?: unknown;
2311+
};
2312+
if (arrowExpr.type !== "ArrowFunctionExpression") {
2313+
return null;
2314+
}
2315+
const paramName = arrowExpr.params?.[0]?.type === "Identifier" ? arrowExpr.params[0].name : null;
2316+
if (!paramName) {
2317+
return null;
2318+
}
2319+
2320+
const body = arrowExpr.body as { type?: string } | undefined;
2321+
if (!body || body.type !== "MemberExpression") {
2322+
return null;
2323+
}
2324+
2325+
const info = extractIndexedThemeLookupInfo(body, paramName);
2326+
if (!info) {
2327+
return null;
2328+
}
2329+
2330+
const resolved = resolveValue({
2331+
kind: "theme",
2332+
path: info.themeObjectPath,
2333+
filePath: state.filePath,
2334+
loc: getNodeLocStart(body as any) ?? undefined,
2335+
});
2336+
if (!resolved) {
2337+
return null;
2338+
}
2339+
2340+
// Register theme imports
2341+
if (resolved.imports) {
2342+
for (const imp of resolved.imports) {
2343+
resolverImports.set(
2344+
JSON.stringify(imp),
2345+
imp as typeof resolverImports extends Map<string, infer V> ? V : never,
2346+
);
2347+
}
2348+
}
2349+
2350+
// Build the indexed expression: resolvedExpr[paramName]
2351+
const resolvedExprAst = parseExpr(resolved.expr);
2352+
const safeParamName = buildSafeIndexedParamName(info.indexPropName, resolvedExprAst);
2353+
const exprSource = `(${resolved.expr})[${safeParamName}]`;
2354+
try {
2355+
const jParse = api.jscodeshift.withParser("tsx");
2356+
const program = jParse(`(${exprSource});`);
2357+
const stmt = program.find(jParse.ExpressionStatement).nodes()[0];
2358+
let parsedExpr = stmt?.expression ?? null;
2359+
while (parsedExpr?.type === "ParenthesizedExpression") {
2360+
parsedExpr = (parsedExpr as { expression: ExpressionKind }).expression;
2361+
}
2362+
// Remove extra.parenthesized flag that causes recast to add parentheses
2363+
const exprWithExtra = parsedExpr as ExpressionKind & {
2364+
extra?: { parenthesized?: boolean; parenStart?: number };
2365+
};
2366+
if (exprWithExtra?.extra?.parenthesized) {
2367+
delete exprWithExtra.extra.parenthesized;
2368+
delete exprWithExtra.extra.parenStart;
2369+
}
2370+
if (!parsedExpr) {
2371+
return null;
2372+
}
2373+
return {
2374+
valueExpr: parsedExpr as ExpressionKind,
2375+
indexPropName: info.indexPropName,
2376+
paramName: safeParamName,
2377+
};
2378+
} catch {
2379+
return null;
2380+
}
22872381
}
22882382

22892383
/**
2290-
* Handles dynamic interpolations inside ::before/::after pseudo-elements by emitting
2291-
* a StyleX dynamic style function whose body wraps the value in the pseudo-element
2292-
* selector.
2384+
* Handles dynamic interpolations inside pseudo-elements (::before / ::after / ::placeholder)
2385+
* by emitting a StyleX dynamic style function whose body wraps the value in the pseudo-element
2386+
* selector. Also handles indexed theme lookups (e.g., props.theme.color[props.$bg]).
22932387
*
22942388
* Example transform:
22952389
* Input: `&::after { background-color: ${(props) => props.$badgeColor}; }`
@@ -2325,60 +2419,104 @@ function tryHandleDynamicPseudoElementStyleFunction(args: InterpolatedDeclaratio
23252419
return false;
23262420
}
23272421

2328-
if (hasThemeAccessInArrowFn(expr)) {
2422+
// For indexed theme lookups (e.g., props.theme.color[props.$bg]), resolve the theme
2423+
// reference and build the indexed expression so the function uses the resolved token.
2424+
const indexedTheme = hasThemeAccessInArrowFn(expr)
2425+
? tryResolveIndexedThemeForPseudoElement(expr, state)
2426+
: null;
2427+
2428+
// Bail on non-indexed theme access (e.g., props.theme.color.primary) — handled elsewhere.
2429+
if (hasThemeAccessInArrowFn(expr) && !indexedTheme) {
23292430
return false;
23302431
}
23312432

2332-
const unwrapped = unwrapArrowFunctionToPropsExpr(j, expr);
2333-
if (!unwrapped) {
2433+
// Bail on CSS shorthand properties for indexed theme lookups.
2434+
// The indexed expression produces a single value that can't be expanded to longhands.
2435+
if (indexedTheme && isCssShorthandProperty(d.property)) {
23342436
return false;
23352437
}
23362438

2337-
const { expr: inlineExpr, propsUsed } = unwrapped;
2338-
2439+
// Bail when the interpolation has surrounding static text and it's an indexed theme lookup.
2440+
// The indexed expression ($colors[param]) cannot be concatenated with a prefix.
23392441
const { prefix, suffix } = extractStaticPartsForDecl(d);
2442+
if (indexedTheme && (prefix || suffix)) {
2443+
return false;
2444+
}
2445+
2446+
let inlineExpr: ExpressionKind;
2447+
let propsUsed: Set<string>;
2448+
let jsxProp: string;
2449+
let isSimpleIdentity: boolean;
2450+
2451+
if (indexedTheme) {
2452+
// Indexed theme: the value expression is the resolved indexed access (e.g., $colors[param]).
2453+
inlineExpr = indexedTheme.valueExpr;
2454+
propsUsed = new Set([indexedTheme.indexPropName]);
2455+
jsxProp = indexedTheme.indexPropName;
2456+
isSimpleIdentity = true;
2457+
} else {
2458+
const unwrapped = unwrapArrowFunctionToPropsExpr(j, expr);
2459+
if (!unwrapped) {
2460+
return false;
2461+
}
2462+
inlineExpr = unwrapped.expr;
2463+
propsUsed = unwrapped.propsUsed;
2464+
// Determine if the expression is a simple identity prop reference (e.g., just `badgeColor`)
2465+
// vs a computed expression (e.g., `tipColor || "black"`, `size * 2`).
2466+
isSimpleIdentity =
2467+
propsUsed.size === 1 &&
2468+
!prefix &&
2469+
!suffix &&
2470+
inlineExpr.type === "Identifier" &&
2471+
propsUsed.has((inlineExpr as { name: string }).name);
2472+
jsxProp = isSimpleIdentity ? [...propsUsed][0]! : "__props";
2473+
}
2474+
23402475
const valueExpr: ExpressionKind =
23412476
prefix || suffix ? buildTemplateWithStaticParts(j, inlineExpr, prefix, suffix) : inlineExpr;
23422477

2343-
// Determine if the expression is a simple identity prop reference (e.g., just `badgeColor`)
2344-
// vs a computed expression (e.g., `tipColor || "black"`, `size * 2`).
2345-
// Simple identity: pass the prop directly as jsxProp.
2346-
// Computed: pass the full expression as callArg to preserve the computation.
2347-
const isSimpleIdentity =
2348-
propsUsed.size === 1 &&
2349-
!prefix &&
2350-
!suffix &&
2351-
inlineExpr.type === "Identifier" &&
2352-
propsUsed.has((inlineExpr as { name: string }).name);
2353-
23542478
const stylexDecls = cssDeclarationToStylexDeclarations(d);
23552479
const pseudoLabel = pseudoElement.replace(/^:+/, "");
2356-
const jsxProp = isSimpleIdentity ? [...propsUsed][0]! : "__props";
23572480

23582481
for (const out of stylexDecls) {
23592482
if (!out.prop) {
23602483
continue;
23612484
}
23622485
const fnKey = styleKeyWithSuffix(styleKeyWithSuffix(decl.styleKey, pseudoLabel), out.prop);
23632486
if (!styleFnDecls.has(fnKey)) {
2364-
// Use the prop name (without $) as parameter for cleaner call sites
2365-
// when possible: styles.badge({ badgeColor }) instead of
2366-
// styles.badge({ backgroundColor: badgeColor })
2367-
const outParamName =
2368-
isSimpleIdentity && jsxProp.startsWith("$")
2487+
// Build parameter name — for indexed theme use the resolved param name,
2488+
// for simple identity use the prop name (without $) for cleaner call sites.
2489+
const outParamName = indexedTheme
2490+
? indexedTheme.paramName
2491+
: isSimpleIdentity && jsxProp.startsWith("$")
23692492
? jsxProp.slice(1)
23702493
: cssPropertyToIdentifier(out.prop, avoidNames);
23712494
const param = j.identifier(outParamName);
2372-
if (isSimpleIdentity && jsxProp !== "__props") {
2495+
2496+
if (indexedTheme) {
2497+
// Use the JSX prop's own type annotation (e.g., Color) when available.
2498+
const propTsType = ctx.findJsxPropTsType(jsxProp);
2499+
(param as { typeAnnotation?: unknown }).typeAnnotation = j.tsTypeAnnotation(
2500+
propTsType && typeof propTsType === "object" && (propTsType as { type?: string }).type
2501+
? (propTsType as ReturnType<typeof j.tsStringKeyword>)
2502+
: j.tsStringKeyword(),
2503+
);
2504+
} else if (isSimpleIdentity && jsxProp !== "__props") {
23732505
ctx.annotateParamFromJsxProp(param, jsxProp);
23742506
} else if (/\.(ts|tsx)$/.test(filePath)) {
23752507
(param as { typeAnnotation?: unknown }).typeAnnotation = j.tsTypeAnnotation(
23762508
j.tsStringKeyword(),
23772509
);
23782510
}
2511+
2512+
// For indexed theme, use the resolved indexed expression directly.
2513+
// For other cases, use the parameter name (potentially wrapped with pseudo/media).
2514+
const innerValueExpr = indexedTheme
2515+
? (cloneAstNode(indexedTheme.valueExpr) as ExpressionKind)
2516+
: j.identifier(outParamName);
23792517
const innerValue = buildPseudoMediaPropValue({
23802518
j,
2381-
valueExpr: j.identifier(outParamName),
2519+
valueExpr: innerValueExpr,
23822520
pseudos,
23832521
media,
23842522
});
@@ -2400,7 +2538,7 @@ function tryHandleDynamicPseudoElementStyleFunction(args: InterpolatedDeclaratio
24002538
}
24012539

24022540
if (isSimpleIdentity) {
2403-
const isOptional = ctx.isJsxPropOptional(jsxProp);
2541+
const isOptional = indexedTheme ? false : ctx.isJsxPropOptional(jsxProp);
24042542
styleFnFromProps.push({
24052543
fnKey,
24062544
jsxProp,

0 commit comments

Comments
 (0)