Skip to content

Commit 41c4059

Browse files
cursoragentskovhus
andcommitted
fix: keep theme bail in module-scoped style function paths
Address review comments: theme-rewritten expressions (theme.X from useTheme()) cannot be placed in module-scoped stylex.create() style functions since the theme variable is only available in the wrapper component body. - Restore bail in pseudo/media path of inline-style-props.ts where the value expression goes into styleFnDecls (module scope) - Restore bail in shouldForwardProp path of rule-interpolated-declaration.ts where the expression also goes into module-scoped style functions - Add regression tests for both scenarios - The non-pseudo/non-media path remains fixed: theme expressions go into callArg (component scope) where useTheme() is available Co-authored-by: Kenneth Skovhus <skovhus@users.noreply.github.com>
1 parent 87fbf8e commit 41c4059

File tree

3 files changed

+58
-21
lines changed

3 files changed

+58
-21
lines changed

src/__tests__/transform.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3597,6 +3597,54 @@ export const App = () => <Box isActive>Hello</Box>;
35973597
expect(result.code).toContain("theme.isDark");
35983598
});
35993599

3600+
it("should bail on theme access inside pseudo/media context (module-scoped style fn)", () => {
3601+
// Theme-rewritten expressions can't be placed in module-scoped stylex.create functions.
3602+
// The `theme` variable from useTheme() is only available in the wrapper component body.
3603+
const source = `
3604+
import styled from "styled-components";
3605+
3606+
const Box = styled.div\`
3607+
&:hover {
3608+
color: \${(props) => props.theme.isDark && props.$isActive ? "red" : "blue"};
3609+
}
3610+
\`;
3611+
3612+
export const App = () => <Box $isActive>Hello</Box>;
3613+
`;
3614+
3615+
const result = transformWithWarnings(
3616+
{ source, path: "theme-pseudo-scope.tsx" },
3617+
{ jscodeshift: j, j, stats: () => {}, report: () => {} },
3618+
{ adapter: fixtureAdapter },
3619+
);
3620+
3621+
// Should bail because theme access in pseudo/media context would put
3622+
// the expression in a module-scoped stylex.create() style function
3623+
expect(result.code).toBeNull();
3624+
});
3625+
3626+
it("should bail on theme access in shouldForwardProp context (module-scoped style fn)", () => {
3627+
const source = `
3628+
import styled from "styled-components";
3629+
3630+
const Box = styled.div\`
3631+
width: \${(props) => props.theme.size * 2}px;
3632+
\`.withConfig({ shouldForwardProp: (p) => p !== "size" });
3633+
3634+
export const App = () => <Box>Hello</Box>;
3635+
`;
3636+
3637+
const result = transformWithWarnings(
3638+
{ source, path: "theme-sfp-scope.tsx" },
3639+
{ jscodeshift: j, j, stats: () => {}, report: () => {} },
3640+
{ adapter: fixtureAdapter },
3641+
);
3642+
3643+
// Should bail because theme access in shouldForwardProp context would put
3644+
// the expression in a module-scoped stylex.create() style function
3645+
expect(result.code).toBeNull();
3646+
});
3647+
36003648
it("should not treat closure variables as destructured props in theme conditionals", () => {
36013649
// When the arrow param is ({ theme }) and the test is `closureVar`,
36023650
// closureVar is NOT a destructured prop — it comes from outer scope.

src/internal/lower-rules/inline-style-props.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@ export function handleInlineStyleValueFromProps(ctx: InlineStyleFromPropsContext
112112
);
113113
}
114114
const valueExprRaw = (() => {
115-
const hasThemeAccess = hasThemeAccessInArrowFn(e);
116-
if (hasThemeAccess && !hasSimpleIdentifierParam(e)) {
115+
// Pseudo/media style functions are module-scoped in stylex.create(),
116+
// so runtime theme values from useTheme() are not available there.
117+
if (hasThemeAccessInArrowFn(e)) {
117118
warnPropInlineStyle(
118119
decl,
119120
"Unsupported prop-based inline style props.theme access is not supported",
@@ -134,14 +135,10 @@ export function handleInlineStyleValueFromProps(ctx: InlineStyleFromPropsContext
134135
setBail();
135136
return null;
136137
}
137-
const baseExpr = hasThemeAccess ? rewritePropsThemeToThemeVar(inlineExpr) : inlineExpr;
138-
if (hasThemeAccess) {
139-
markDeclNeedsUseThemeHook(decl);
140-
}
141138
const { prefix, suffix } = extractStaticPartsForDecl(d);
142139
return prefix || suffix
143-
? buildTemplateWithStaticParts(j, baseExpr, prefix, suffix)
144-
: baseExpr;
140+
? buildTemplateWithStaticParts(j, inlineExpr, prefix, suffix)
141+
: inlineExpr;
145142
})();
146143
if (!valueExprRaw) {
147144
return true;

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

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1964,8 +1964,9 @@ export function handleInterpolatedDeclaration(args: InterpolatedDeclarationConte
19641964
bail = true;
19651965
break;
19661966
}
1967-
const hasThemeAccess = hasThemeAccessInArrowFn(e);
1968-
if (hasThemeAccess && e.params?.[0]?.type !== "Identifier") {
1967+
// shouldForwardProp style functions are module-scoped in stylex.create(),
1968+
// so runtime theme values from useTheme() are not available there.
1969+
if (hasThemeAccessInArrowFn(e)) {
19691970
warnPropInlineStyle(
19701971
decl,
19711972
"Unsupported prop-based inline style props.theme access is not supported",
@@ -1977,16 +1978,12 @@ export function handleInterpolatedDeclaration(args: InterpolatedDeclarationConte
19771978
}
19781979
const propsUsed = collectPropsFromArrowFn(e);
19791980
for (const propName of propsUsed) {
1980-
if (hasThemeAccess && propName === "theme") {
1981-
continue;
1982-
}
19831981
ensureShouldForwardPropDrop(decl, propName);
19841982
}
19851983
// Try to unwrap props access (props.$x → $x) for cleaner style functions.
19861984
// When only one transient prop is used, emit a single-param function
19871985
// (e.g., ($size) => ...) instead of (props) => ..., enabling consolidation.
1988-
// Skip unwrapping when theme is accessed — theme refs need full inlining + rewriting.
1989-
const unwrapped = hasThemeAccess ? null : unwrapArrowFunctionToPropsExpr(j, e);
1986+
const unwrapped = unwrapArrowFunctionToPropsExpr(j, e);
19901987
if (unwrapped && unwrapped.propsUsed.size === 1) {
19911988
const singleProp = [...unwrapped.propsUsed][0]!;
19921989
propsParam = j.identifier(singleProp);
@@ -2007,12 +2004,7 @@ export function handleInterpolatedDeclaration(args: InterpolatedDeclarationConte
20072004
bail = true;
20082005
break;
20092006
}
2010-
baseExpr = hasThemeAccess
2011-
? rewritePropsThemeToThemeVar(inlineExpr as ExpressionKind)
2012-
: inlineExpr;
2013-
if (hasThemeAccess) {
2014-
markDeclNeedsUseThemeHook(decl);
2015-
}
2007+
baseExpr = inlineExpr;
20162008
}
20172009
}
20182010
// Build template literal when there's static prefix/suffix (e.g., `${...}ms`)

0 commit comments

Comments
 (0)