Skip to content

Commit 794b5d4

Browse files
skovhusclaude
andauthored
refactor: reduce test case duplication by merging 12 near-duplicate tests (#347)
* refactor: reduce test case duplication by merging 12 near-duplicate tests Consolidate test cases that test the same patterns into existing cases, preserving all unique patterns as additional components/variations: - conditional-basic ← conditional-ternaryCssBlock (empty string ternary) - conditional-nullishCoalescing ← conditional-nullishCoalescingWithUnit (numeric+unit) - conditional-logicalAnd ← conditional-logicalAndTemplateLiteral (theme templates) - conditional-runtimeCallBranch ← conditional-runtimeCallThemeBool (plain fn call) - interpolation-arrowFunction ← interpolation-arrowBlockBody (block body syntax) - attrs-labelAs ← attrs-asRefType (variant prop + ref typing) - attrs-tabIndex ← attrs-tabIndexInStyle (tabIndex used in CSS) - selector-pseudoExpand ← selector-pseudoExpandInline (single-use inlining) - selector-sibling ← selector-adjacentSiblingDestructure + selector-siblingInterpolated - selector-dynamicPseudoElement ← selector-dynamicPlaceholder (::placeholder) - transientProp-notForwarded ← transientProp-toDom (DOM element filtering) https://claude.ai/code/session_014eC8v66Xj9xpyaZbYpFT7G * fix: use per-component defineMarker() for scoped sibling selectors siblingBefore(":is(*)") with file-global defaultMarker() caused sibling styles to leak across component boundaries when multiple sibling-using components appeared as siblings in the same render tree. The first Row was incorrectly getting margin-top:16px because ThingThemed (a different component) preceded it and shared the same default marker. Fix: generate a defineMarker() per component in a .stylex sidecar file and pass it as the second argument to siblingBefore(":is(*)", Marker). This scopes sibling matching to only same-component siblings. https://claude.ai/code/session_014eC8v66Xj9xpyaZbYpFT7G * refactor: collect markers before pushing to avoid mutation during iteration https://claude.ai/code/session_014eC8v66Xj9xpyaZbYpFT7G * chore: regenerate all test case outputs after rebase with main https://claude.ai/code/session_014eC8v66Xj9xpyaZbYpFT7G --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent af08afe commit 794b5d4

File tree

64 files changed

+630
-951
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+630
-951
lines changed

eslint.config.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,9 @@ export default [
5353
// Dynamic style function parameters: the rule cannot validate runtime
5454
// values passed as arrow-function params in stylex.create().
5555
files: [
56-
"test-cases/conditional-nullishCoalescingWithUnit.output.tsx",
56+
"test-cases/conditional-nullishCoalescing.output.tsx",
5757
"test-cases/conditional-runtimeCallBranch.output.tsx",
5858
"test-cases/conditional-runtimeCallLocal.output.tsx",
59-
"test-cases/conditional-runtimeCallThemeBool.output.tsx",
6059
"test-cases/helper-callPropArg.output.tsx",
6160
"test-cases/helper-memberCalleeMultiArg.output.tsx",
6261
"test-cases/interpolation-destructuredDefaults.output.tsx",

src/internal/emit-wrappers.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ export function emitWrappers(args: {
2424
themeHook?: ThemeHookConfig;
2525
emptyStyleKeys?: Set<string>;
2626
ancestorSelectorParents?: Set<string>;
27+
crossFileMarkers?: Map<string, string>;
28+
siblingMarkerKeys?: Set<string>;
2729
useSxProp: boolean;
2830
}): void {
2931
const {
@@ -39,6 +41,8 @@ export function emitWrappers(args: {
3941
themeHook,
4042
emptyStyleKeys,
4143
ancestorSelectorParents,
44+
crossFileMarkers,
45+
siblingMarkerKeys,
4246
useSxProp,
4347
} = args;
4448

@@ -60,6 +64,8 @@ export function emitWrappers(args: {
6064
themeHook: themeHook ?? DEFAULT_THEME_HOOK,
6165
emptyStyleKeys,
6266
ancestorSelectorParents,
67+
crossFileMarkers,
68+
siblingMarkerKeys,
6369
useSxProp,
6470
});
6571

src/internal/emit-wrappers/style-merger.ts

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ export function emitStyleMerging(args: {
7171
| "stylesIdentifier"
7272
| "emptyStyleKeys"
7373
| "ancestorSelectorParents"
74+
| "crossFileMarkers"
75+
| "siblingMarkerKeys"
7476
| "emitTypes"
7577
| "useSxProp"
7678
>;
@@ -100,8 +102,15 @@ export function emitStyleMerging(args: {
100102
isIntrinsicElement = true,
101103
} = args;
102104

103-
const { styleMerger, emptyStyleKeys, stylesIdentifier, ancestorSelectorParents, emitTypes } =
104-
emitter;
105+
const {
106+
styleMerger,
107+
emptyStyleKeys,
108+
stylesIdentifier,
109+
ancestorSelectorParents,
110+
crossFileMarkers,
111+
siblingMarkerKeys,
112+
emitTypes,
113+
} = emitter;
105114

106115
const styleArgs = filterEmptyStyleArgs({
107116
styleArgs: rawStyleArgs,
@@ -110,13 +119,29 @@ export function emitStyleMerging(args: {
110119
ancestorSelectorParents,
111120
});
112121

113-
// Add stylex.defaultMarker() when any style arg references an ancestor selector parent.
114-
// This is needed for merger/verbose paths that bypass the postProcessTransformedAst traversal.
122+
// Add a marker when any style arg references an ancestor selector parent.
123+
// Sibling markers (from crossFileMarkers) REPLACE defaultMarker() — they scope sibling
124+
// matching to the component. Cross-file markers coexist with defaultMarker().
115125
if (ancestorSelectorParents && ancestorSelectorParents.size > 0) {
116-
const needsMarker = styleArgs.some((arg) =>
117-
hasStyleArgKey(arg, stylesIdentifier, ancestorSelectorParents),
118-
);
119-
if (needsMarker) {
126+
let needsDefaultMarker = false;
127+
const pendingMarkers: ExpressionKind[] = [];
128+
for (const arg of styleArgs) {
129+
const key = getStyleArgKey(arg, stylesIdentifier);
130+
if (!key || !ancestorSelectorParents.has(key)) {
131+
continue;
132+
}
133+
const markerVarName = crossFileMarkers.get(key);
134+
if (markerVarName) {
135+
pendingMarkers.push(j.identifier(markerVarName));
136+
}
137+
// Need defaultMarker() when there's no scoped marker, or when a cross-file
138+
// marker coexists with an ancestor selector (defaultMarker serves the ancestor).
139+
if (!markerVarName || !siblingMarkerKeys.has(key)) {
140+
needsDefaultMarker = true;
141+
}
142+
}
143+
styleArgs.push(...pendingMarkers);
144+
if (needsDefaultMarker) {
120145
styleArgs.push(
121146
j.callExpression(
122147
j.memberExpression(j.identifier("stylex"), j.identifier("defaultMarker")),
@@ -554,15 +579,3 @@ function getStyleArgKey(node: ExpressionKind, stylesIdentifier: string): string
554579
}
555580
return null;
556581
}
557-
558-
/**
559-
* Returns `true` when `node` is `<stylesIdentifier>.<key>` and the key is in the given set.
560-
*/
561-
function hasStyleArgKey(
562-
node: ExpressionKind,
563-
stylesIdentifier: string,
564-
keys: Set<string>,
565-
): boolean {
566-
const key = getStyleArgKey(node, stylesIdentifier);
567-
return !!(key && keys.has(key));
568-
}

src/internal/emit-wrappers/wrapper-emitter.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ type WrapperEmitterArgs = {
4646
themeHook: ThemeHookConfig;
4747
emptyStyleKeys?: Set<string>;
4848
ancestorSelectorParents?: Set<string>;
49+
/** Maps styleKey → marker variable name for scoped markers (sibling + cross-file) */
50+
crossFileMarkers?: Map<string, string>;
51+
/** Style keys that use sibling markers (scoped marker replaces defaultMarker) */
52+
siblingMarkerKeys?: Set<string>;
4953
useSxProp: boolean;
5054
};
5155

@@ -62,6 +66,8 @@ export class WrapperEmitter {
6266
readonly themeHook: ThemeHookConfig;
6367
readonly emptyStyleKeys: Set<string>;
6468
readonly ancestorSelectorParents: Set<string>;
69+
readonly crossFileMarkers: Map<string, string>;
70+
readonly siblingMarkerKeys: Set<string>;
6571
readonly useSxProp: boolean;
6672

6773
// For plain JS/JSX and Flow transforms, skip emitting TS syntax entirely for now.
@@ -87,6 +93,8 @@ export class WrapperEmitter {
8793
this.themeHook = args.themeHook;
8894
this.emptyStyleKeys = args.emptyStyleKeys ?? new Set<string>();
8995
this.ancestorSelectorParents = args.ancestorSelectorParents ?? new Set<string>();
96+
this.crossFileMarkers = args.crossFileMarkers ?? new Map<string, string>();
97+
this.siblingMarkerKeys = args.siblingMarkerKeys ?? new Set<string>();
9098
this.useSxProp = args.useSxProp;
9199
this.emitTypes = this.filePath.endsWith(".ts") || this.filePath.endsWith(".tsx");
92100
}

src/internal/lower-rules.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export function lowerRules(ctx: TransformContext): {
2222
ancestorSelectorParents: Set<string>;
2323
usedCssHelperFunctions: Set<string>;
2424
crossFileMarkers: Map<string, string>;
25+
siblingMarkerKeys: Set<string>;
2526
bail: boolean;
2627
} {
2728
const state = createLowerRulesState(ctx);
@@ -113,14 +114,18 @@ export function lowerRules(ctx: TransformContext): {
113114
}
114115
}
115116

116-
// Derive cross-file markers from relation overrides (single source of truth).
117-
// Only include markers for parents that actually need them.
117+
// Derive markers from relation overrides and sibling selectors.
118+
// Cross-file overrides use markers for parents that need them;
119+
// sibling selectors use per-component markers for scoped matching.
118120
const crossFileMarkers = new Map<string, string>();
119121
for (const o of state.relationOverrides) {
120122
if (o.crossFile && o.markerVarName && parentsNeedingMarker.has(o.parentStyleKey)) {
121123
crossFileMarkers.set(o.parentStyleKey, o.markerVarName);
122124
}
123125
}
126+
for (const [styleKey, markerName] of state.siblingMarkerNames) {
127+
crossFileMarkers.set(styleKey, markerName);
128+
}
124129

125130
// Filter ancestorSelectorParents to only parents needing markers.
126131
// Parents without pseudo conditions don't need markers — their override
@@ -140,6 +145,7 @@ export function lowerRules(ctx: TransformContext): {
140145
ancestorSelectorParents: filteredAncestorParents,
141146
usedCssHelperFunctions: state.usedCssHelperFunctions,
142147
crossFileMarkers,
148+
siblingMarkerKeys: new Set(state.siblingMarkerNames.keys()),
143149
bail: state.bail,
144150
};
145151
}

src/internal/lower-rules/process-rules.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,7 +1869,7 @@ function getOrCreateComputedMediaEntry(prop: string, ctx: DeclProcessingState) {
18691869

18701870
/**
18711871
* Handles a self-referencing sibling selector (`& + &` or `& ~ &`) by processing
1872-
* declarations and storing them as computed keys using `stylex.when.siblingBefore(':is(*)')`.
1872+
* declarations and storing them as computed keys using `stylex.when.siblingBefore(':is(*)', Marker)`.
18731873
*
18741874
* **Semantic approximation:** CSS `& + &` targets only the *immediately adjacent*
18751875
* sibling, while `stylex.when.siblingBefore()` generates a general sibling rule
@@ -1878,11 +1878,10 @@ function getOrCreateComputedMediaEntry(prop: string, ctx: DeclProcessingState) {
18781878
* In practice this rarely matters because `& + &` is almost always used with
18791879
* consecutive homogeneous lists. For `& ~ &`, the mapping is semantically exact.
18801880
*
1881-
* Uses `defaultMarker()` (via `ancestorSelectorParents`) instead of per-component
1882-
* `defineMarker()`, avoiding the `.stylex` file requirement. Because the marker
1883-
* is file-global rather than component-scoped, styles from one component could
1884-
* theoretically leak to another if both use sibling selectors and appear as
1885-
* siblings in the same render tree.
1881+
* Uses per-component `defineMarker()` (emitted in a `.stylex` sidecar file) so that
1882+
* sibling matching is component-scoped. Without a scoped marker, `defaultMarker()` is
1883+
* file-global and styles from one component could leak to another if both use sibling
1884+
* selectors and appear as siblings in the same render tree.
18861885
*
18871886
* **`:is(*)` workaround:** The StyleX Babel plugin mandates a pseudo argument
18881887
* starting with `:` for `siblingBefore()`. `:is(*)` is a universal match with
@@ -1911,12 +1910,18 @@ function handleSiblingSelector(
19111910
});
19121911
}
19131912

1914-
// Add to ancestorSelectorParents so defaultMarker() is injected into stylex.props() calls.
1913+
// Add to ancestorSelectorParents so the marker is injected into stylex.props() calls.
19151914
// Also add to siblingMarkerParents to distinguish from forward/reverse selectors —
19161915
// sibling markers are always needed (stylex.when.siblingBefore() references them).
19171916
ancestorSelectorParents.add(decl.styleKey);
19181917
state.siblingMarkerParents.add(decl.styleKey);
19191918

1919+
// Register a per-component marker for scoped sibling matching.
1920+
// The marker variable (e.g. "ThingMarker") is emitted as defineMarker() in a
1921+
// sidecar .stylex file and passed as the second argument to siblingBefore().
1922+
const markerVarName = state.siblingMarkerNames.get(decl.styleKey) ?? `${decl.localName}Marker`;
1923+
state.siblingMarkerNames.set(decl.styleKey, markerVarName);
1924+
19201925
// Process declarations into a temporary bucket using the shared helper
19211926
const bucket: Record<string, unknown> = {};
19221927
const result = processDeclarationsIntoBucket(
@@ -1938,14 +1943,15 @@ function handleSiblingSelector(
19381943
return "break";
19391944
}
19401945

1941-
// Build a fresh stylex.when.siblingBefore(':is(*)') AST node per property.
1946+
// Build a fresh stylex.when.siblingBefore(':is(*)', Marker) AST node per property.
1947+
// The second argument scopes sibling matching to the component's own marker.
19421948
const makeSiblingKeyExpr = () =>
19431949
j.callExpression(
19441950
j.memberExpression(
19451951
j.memberExpression(j.identifier("stylex"), j.identifier("when")),
19461952
j.identifier("siblingBefore"),
19471953
),
1948-
[j.literal(":is(*)")],
1954+
[j.literal(":is(*)"), j.identifier(markerVarName)],
19491955
);
19501956

19511957
// Wrap values in media/container condition objects when inside an @media or @container at-rule

src/internal/lower-rules/state.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ export function createLowerRulesState(ctx: TransformContext) {
7171
const relationOverrides: RelationOverride[] = [];
7272
const ancestorSelectorParents = new Set<string>();
7373
const siblingMarkerParents = new Set<string>();
74+
/** Maps styleKey → marker variable name for sibling selectors (e.g. "thing" → "ThingMarker") */
75+
const siblingMarkerNames = new Map<string, string>();
7476
// Map<overrideStyleKey, Map<pseudo|null, Record<prop, value>>>
7577
// null key = base styles, string key = pseudo styles (e.g., ":hover", ":focus-visible")
7678
const relationOverridePseudoBuckets = new Map<
@@ -263,6 +265,7 @@ export function createLowerRulesState(ctx: TransformContext) {
263265
relationOverrides,
264266
ancestorSelectorParents,
265267
siblingMarkerParents,
268+
siblingMarkerNames,
266269
relationOverridePseudoBuckets,
267270
childPseudoMarkers,
268271
cssHelperValuesByKey,

src/internal/transform-context.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,10 @@ export class TransformContext {
9696
crossFileSelectorUsages?: import("./transform-types.js").CrossFileSelectorUsage[];
9797
/** Component names in this file that need a global selector bridge className */
9898
bridgeComponentNames?: Set<string>;
99-
/** Marker variable names generated for cross-file parent components (parentStyleKey → markerName) */
99+
/** Marker variable names generated for cross-file parent components and sibling selectors (parentStyleKey → markerName) */
100100
crossFileMarkers?: Map<string, string>;
101+
/** Style keys that use sibling markers (scoped marker replaces defaultMarker) */
102+
siblingMarkerKeys?: Set<string>;
101103
/** Content for the sidecar .stylex.ts file (defineMarker declarations), populated by emitStylesStep */
102104
sidecarStylexContent?: string;
103105
/** Bridge components emitted for unconverted consumer selectors. */

src/internal/transform-steps/emit-wrappers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export function emitWrappersStep(ctx: TransformContext): StepResult {
2929
themeHook: ctx.adapter.themeHook,
3030
emptyStyleKeys: ctx.emptyStyleKeys,
3131
ancestorSelectorParents: ctx.ancestorSelectorParents,
32+
crossFileMarkers: ctx.crossFileMarkers,
33+
siblingMarkerKeys: ctx.siblingMarkerKeys,
3234
useSxProp: ctx.adapter.useSxProp,
3335
});
3436

src/internal/transform-steps/lower-rules.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export function lowerRulesStep(ctx: TransformContext): StepResult {
2121
ctx.relationOverrides = lowered.relationOverrides;
2222
ctx.ancestorSelectorParents = lowered.ancestorSelectorParents;
2323
ctx.crossFileMarkers = lowered.crossFileMarkers;
24+
ctx.siblingMarkerKeys = lowered.siblingMarkerKeys;
2425

2526
if (lowered.bail || ctx.resolveValueBailRef.value) {
2627
return returnResult({ code: null, warnings: ctx.warnings }, "bail");

0 commit comments

Comments
 (0)