Skip to content

Commit fb9914c

Browse files
skovhusclaude
andcommitted
fix: merge extraClassNames into stylex.props spread for useSxProp: false
When useSxProp is false, the inline path emits {...stylex.props(...)}. Previously, extraClassNames were added as a separate className= attribute after the spread, which overrides the spread's className and loses the StyleX classes entirely. Now, for the non-sx path, extraClassNames are folded into classNameAttr so buildInlineMergeCall merges them properly. For the sx path (sx and className are independent), the separate className attribute is preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f2954ad commit fb9914c

File tree

2 files changed

+124
-19
lines changed

2 files changed

+124
-19
lines changed

src/__tests__/transform.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5256,3 +5256,74 @@ export const App = () => <Box $spacing="sm">Content</Box>;
52565256
}
52575257
});
52585258
});
5259+
5260+
describe("extraClassNames with useSxProp: false", () => {
5261+
it("should merge CSS module className into stylex.props spread instead of adding duplicate className", () => {
5262+
const source = `
5263+
import styled from "styled-components";
5264+
import { draggableRegion } from "./lib/helpers";
5265+
5266+
const DraggableBar = styled.div\`
5267+
pointer-events: all;
5268+
\${draggableRegion(true)};
5269+
\`;
5270+
5271+
export function App() {
5272+
return <DraggableBar>Draggable</DraggableBar>;
5273+
}
5274+
`;
5275+
5276+
const adapterWithNoSxProp = {
5277+
externalInterface() {
5278+
return { styles: false, as: false, ref: false };
5279+
},
5280+
resolveValue() {
5281+
return undefined;
5282+
},
5283+
resolveCall(ctx: Parameters<NonNullable<Adapter["resolveCall"]>>[0]) {
5284+
if (ctx.calleeImportedName === "draggableRegion") {
5285+
return {
5286+
extraClassNames: [
5287+
{
5288+
expr: "electronStyles.draggableRegionDisableChildren",
5289+
imports: [
5290+
{
5291+
from: { kind: "specifier" as const, value: "./lib/electronMixins.module.css" },
5292+
names: [{ imported: "default", local: "electronStyles" }],
5293+
},
5294+
],
5295+
},
5296+
],
5297+
};
5298+
}
5299+
return undefined;
5300+
},
5301+
resolveSelector() {
5302+
return undefined;
5303+
},
5304+
styleMerger: null,
5305+
useSxProp: false,
5306+
} satisfies Adapter;
5307+
5308+
const result = transformWithWarnings(
5309+
{
5310+
source,
5311+
path: join(testCasesDir, "mixin-extraClassNames.input.tsx"),
5312+
},
5313+
{ jscodeshift: j, j, stats: () => {}, report: () => {} },
5314+
{ adapter: adapterWithNoSxProp },
5315+
);
5316+
5317+
expect(result.code).not.toBeNull();
5318+
// With useSxProp: false, the output uses {...stylex.props(...)}.
5319+
// The CSS module className must be folded into the merge, not added as a
5320+
// separate className attribute that would override the spread's className.
5321+
// A duplicate className= after {...stylex.props(...)} causes the element to
5322+
// lose its StyleX classes entirely.
5323+
expect(result.code).toContain("stylex.props");
5324+
expect(result.code).toContain("electronStyles");
5325+
// The className from CSS module should NOT be a standalone JSX attribute
5326+
// following the spread — it must be merged.
5327+
expect(result.code).not.toMatch(/\{\.\.\.stylex\.props\([^)]+\)\}\s*className=/);
5328+
});
5329+
});

src/internal/transform-steps/rewrite-jsx.ts

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -737,11 +737,32 @@ export function rewriteJsxStep(ctx: TransformContext): StepResult {
737737
styleAttr = null;
738738
}
739739

740+
// Build extra className expression from CSS module classes (if any).
741+
const extraClassNameExpr =
742+
decl.extraClassNames && decl.extraClassNames.length > 0
743+
? buildExtraClassNameExpr(j, decl.extraClassNames)
744+
: undefined;
745+
740746
// Build final rest with stylex.props inserted after last spread.
741747
// For inlined components with className/style, use adapter-configured
742748
// merger behavior (or verbose fallback when no merger is configured).
743-
const needsMerge = classNameAttr !== null || styleAttr !== null;
744749
const isIntrinsicTag = /^[a-z]/.test(finalTag) && !finalTag.includes(".");
750+
751+
// When NOT using sx prop, CSS module classNames must be merged into
752+
// the stylex.props spread (via classNameAttr) to avoid a duplicate
753+
// className attribute that would override the spread's className.
754+
// When using sx prop, sx and className are independent attributes.
755+
let effectiveClassNameAttr = classNameAttr;
756+
if (extraClassNameExpr && !ctx.adapter.useSxProp) {
757+
// Synthesize a JSX className attribute so buildInlineMergeCall
758+
// folds the CSS module class into the spread merge.
759+
effectiveClassNameAttr = j.jsxAttribute(
760+
j.jsxIdentifier("className"),
761+
j.jsxExpressionContainer(extraClassNameExpr),
762+
);
763+
}
764+
765+
const needsMerge = effectiveClassNameAttr !== null || styleAttr !== null;
745766
const useSxProp = ctx.adapter.useSxProp && !needsMerge && isIntrinsicTag;
746767
const stylexAttr = useSxProp
747768
? (() => {
@@ -756,7 +777,7 @@ export function rewriteJsxStep(ctx: TransformContext): StepResult {
756777
? buildInlineMergeCall(
757778
j,
758779
styleArgs,
759-
classNameAttr,
780+
effectiveClassNameAttr,
760781
styleAttr,
761782
ctx.adapter.styleMerger?.functionName,
762783
)
@@ -765,25 +786,16 @@ export function rewriteJsxStep(ctx: TransformContext): StepResult {
765786
[...styleArgs],
766787
),
767788
);
768-
// Emit extraClassNames as a className attribute (CSS module classes)
789+
790+
// For sx prop mode, emit extraClassNames as a separate className attribute
791+
// (sx and className are independent and don't conflict).
769792
const extraClassNameAttrs: typeof keptRestAfterVariants = [];
770-
if (decl.extraClassNames && decl.extraClassNames.length > 0) {
771-
const classNameExprs = decl.extraClassNames.map((cn) => cn.expr);
772-
const classNameExpr =
773-
classNameExprs.length === 1 && classNameExprs[0]
774-
? classNameExprs[0]
775-
: (() => {
776-
// Multiple: join with template literal `${a} ${b}`
777-
const qs: ReturnType<typeof j.templateElement>[] = [];
778-
for (let i = 0; i <= classNameExprs.length; i++) {
779-
const isLast = i === classNameExprs.length;
780-
const raw = i === 0 || isLast ? "" : " ";
781-
qs.push(j.templateElement({ raw, cooked: raw }, isLast));
782-
}
783-
return j.templateLiteral(qs, classNameExprs);
784-
})();
793+
if (extraClassNameExpr && useSxProp) {
785794
extraClassNameAttrs.push(
786-
j.jsxAttribute(j.jsxIdentifier("className"), j.jsxExpressionContainer(classNameExpr)),
795+
j.jsxAttribute(
796+
j.jsxIdentifier("className"),
797+
j.jsxExpressionContainer(extraClassNameExpr),
798+
),
787799
);
788800
}
789801

@@ -935,6 +947,28 @@ function extractJsxAttrValueExpr(
935947
return undefined;
936948
}
937949

950+
/**
951+
* Builds a single expression from extra className entries (CSS module classes).
952+
* Single entry: returns the expression directly.
953+
* Multiple entries: joins with a template literal `${a} ${b}`.
954+
*/
955+
function buildExtraClassNameExpr(
956+
j: TransformContext["j"]["jscodeshift"],
957+
extraClassNames: NonNullable<StyledDecl["extraClassNames"]>,
958+
): ExpressionKind {
959+
const exprs = extraClassNames.map((cn) => cn.expr);
960+
if (exprs.length === 1 && exprs[0]) {
961+
return exprs[0];
962+
}
963+
const qs: ReturnType<typeof j.templateElement>[] = [];
964+
for (let i = 0; i <= exprs.length; i++) {
965+
const isLast = i === exprs.length;
966+
const raw = i === 0 || isLast ? "" : " ";
967+
qs.push(j.templateElement({ raw, cooked: raw }, isLast));
968+
}
969+
return j.templateLiteral(qs, exprs);
970+
}
971+
938972
/**
939973
* Finds the combined style key matching the consumed props at a JSX call site.
940974
* Returns the style key if a matching combination exists, or undefined otherwise.

0 commit comments

Comments
 (0)