Skip to content

Commit a2aa553

Browse files
committed
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
1 parent e8818d2 commit a2aa553

File tree

3 files changed

+70
-1
lines changed

3 files changed

+70
-1
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/value-patterns.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55
import type { StyledDecl } from "../transform-types.js";
66
import type { ExpressionKind, StyleFnFromPropsEntry } from "./decl-types.js";
7-
import { cssDeclarationToStylexDeclarations } from "../css-prop-mapping.js";
7+
import { cssDeclarationToStylexDeclarations, isCssShorthandProperty } from "../css-prop-mapping.js";
88
import { extractStaticPartsForDecl } from "./interpolations.js";
99
import { buildTemplateWithStaticParts } from "./inline-styles.js";
1010
import { ensureShouldForwardPropDrop, literalToStaticValue } from "./types.js";
@@ -572,6 +572,12 @@ export const createValuePatternHandlers = (ctx: ValuePatternContext) => {
572572
if (!d.property) {
573573
return false;
574574
}
575+
// Bail on CSS shorthand properties (padding, margin, border, background).
576+
// The indexed theme expression emits a single value which can't be expanded
577+
// into longhand properties, and StyleX forbids shorthands.
578+
if (isCssShorthandProperty(d.property)) {
579+
return false;
580+
}
575581
// Skip media/attr buckets for now; these require more complex wiring.
576582
if (opts.media || opts.attrTarget) {
577583
return false;

0 commit comments

Comments
 (0)