From 13263016367b7f3ff7a0868f7f37053864ee6659 Mon Sep 17 00:00:00 2001 From: "Henry Q. Dineen" Date: Wed, 3 Jun 2026 18:00:48 -0400 Subject: [PATCH] [babel-plugin] only substitute referenced constants in processStylexRules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit processStylexRules looped over every constant in constsMap and replaced it in every non-constant rule -- O(rules x constsMap). On a real ~15.7k-rule, ~3.3k-const build that was ~7.5s, almost entirely this loop. Substituting only the references actually present in each rule drops it to ~28ms (~27% of rules reference a constant, ~1.1 distinct refs each). It also makes override resolution deterministic. A defineConsts value can be a var(...); used as a property key it overrides that variable in scope (#1219), and processStylexRules rewrites the declared key to the variable the const aliases. The previous loop applied that rewrite cumulatively across all of constsMap, so for chained aliases it followed the chain a variable distance depending on order. Now: - value substitution runs once per referenced var (values are pre-resolved by resolveConstant; only var(--x, fallback), which is overridable, is left); and - the override-key rewrite is a single pass that resolves each declaration one independent step, so it cannot cascade across declarations. The per-rule scan matches any custom-property key, including non-ASCII (e.g. --café), so those substitute correctly. resolveConstant is left untouched, so it still only pre-collapses ASCII alias chains -- a non-ASCII const->const chain resolves a single step here rather than through to its terminal. That's a pre-existing limitation we noticed but did not change (terminal non-ASCII keys resolve fine; escaped-delimiter names like --foo\:bar are also out of scope). Tests: bare vs fallback chain resolution; single- and multi-declaration override rewrites resolve one step and are order-independent; non-ASCII key (terminal, plus the unresolved-chain limitation); no-reference / no-constant paths; one end-to-end cross-file case (a @media breakpoint const consumed from a separate module via a real import) through transform(). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../__tests__/transform-process-test.js | 297 ++++++++++++++++++ packages/@stylexjs/babel-plugin/src/index.js | 70 +++-- 2 files changed, 344 insertions(+), 23 deletions(-) diff --git a/packages/@stylexjs/babel-plugin/__tests__/transform-process-test.js b/packages/@stylexjs/babel-plugin/__tests__/transform-process-test.js index 58c30ac1f..85a39f33c 100644 --- a/packages/@stylexjs/babel-plugin/__tests__/transform-process-test.js +++ b/packages/@stylexjs/babel-plugin/__tests__/transform-process-test.js @@ -1230,4 +1230,301 @@ describe('@stylexjs/babel-plugin', () => { `); }); }); + + describe('[transform] processStylexRules constant substitution', () => { + const constRule = (key, constVal) => [key, { constKey: key, constVal }, 0]; + const cssRule = (className, ltr) => [className, { ltr, rtl: null }, 3000]; + + test('substitutes referenced constants and ignores unreferenced ones', () => { + expect( + stylexPlugin.processStylexRules( + [ + constRule('a', 'red'), + constRule('b', 'blue'), + cssRule('x1', '.x1{color:var(--a)}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x1{color:red}"'); + }); + + test('resolves a const aliasing an external var to that var reference', () => { + // The real-world pattern: a const aliasing a CSS variable defined outside + // StyleX (e.g. a design-system var). It stays a `var()` the browser reads + // at runtime -- StyleX never defines `--color-primary` itself. + expect( + stylexPlugin.processStylexRules( + [ + constRule('primary', 'var(--color-primary)'), + cssRule('x1', '.x1{color:var(--primary)}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x1{color:var(--color-primary)}"'); + }); + + test('resolves a bare-ref chain through to the terminal value', () => { + // resolveConstant pre-collapses bare-ref chains, so this resolves in one hop. + expect( + stylexPlugin.processStylexRules( + [ + constRule('a', 'var(--b)'), + constRule('b', 'green'), + cssRule('x1', '.x1{color:var(--a)}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x1{color:green}"'); + }); + + test('leaves a fallback-form chain at the first hop', () => { + // Atypical (real consts alias one external var, see above) -- a const + // aliasing another const via a fallback form. resolveConstant doesn't + // collapse fallback refs, so it stops at the first hop, the shape the + // override tests below exercise. + expect( + stylexPlugin.processStylexRules( + [ + constRule('a', 'var(--b, blue)'), + constRule('b', 'green'), + cssRule('x1', '.x1{color:var(--a)}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x1{color:var(--b, blue)}"'); + }); + + test('rewrites a const-key declaration to the var it aliases', () => { + // A const used as a property key emits `--orange:`, rewritten to the var + // it aliases. Also guards that the scan matches a bare `--key:`, not only + // `var(--key)`. + expect( + stylexPlugin.processStylexRules( + [ + constRule('orange', 'var(--orange-theme-color)'), + cssRule('cls', '.cls{--orange:red}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".cls{--orange-theme-color:red}"'); + }); + + test('substitutes a const used as both value and declaration in one rule', () => { + expect( + stylexPlugin.processStylexRules( + [ + constRule('orange', 'var(--orange-theme-color)'), + cssRule('cls', '.cls{color:var(--orange);--orange:red}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot( + '".cls{color:var(--orange-theme-color);--orange-theme-color:red}"', + ); + }); + + test('parses the target var from an alias with a fallback', () => { + // `--orange` aliases `--orange-theme-color` (with a `blue` fallback). As a + // declaration key it rewrites to that var name alone -- `--orange-theme-color:`, + // dropping the fallback (a `--x, blue:` property would be nonsense). + expect( + stylexPlugin.processStylexRules( + [ + constRule('orange', 'var(--orange-theme-color, blue)'), + cssRule('cls', '.cls{--orange:red}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".cls{--orange-theme-color:red}"'); + }); + + test('rewrites a chained const-key declaration a single step', () => { + // Atypical multi-hop const->const->const chain (real consts alias one + // external var). `--primary` aliases `--brandPrimary` aliases `--blue500`; + // the value resolves one link (to `var(--brandPrimary, …)`), so the key + // rewrite lands one link too -- on `--brandPrimary`, the var the value + // reads. Setting `--blue500` would be a no-op. + expect( + stylexPlugin.processStylexRules( + [ + constRule('blue500', '#3b82f6'), + constRule('brandPrimary', 'var(--blue500, #3b82f6)'), + constRule('primary', 'var(--brandPrimary, #3b82f6)'), + cssRule('cls', '.cls{color:var(--primary);--primary:transparent}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot( + '".cls{color:var(--brandPrimary, #3b82f6);--brandPrimary:transparent}"', + ); + }); + + test('const-key declaration rewrite is independent of constant order', () => { + // Either order resolves `--primary` to `--brandPrimary` (the var its value + // reads), never on to `--blue500`. + const blue500 = constRule('blue500', '#3b82f6'); + const brandPrimary = constRule('brandPrimary', 'var(--blue500, #3b82f6)'); + const primary = constRule('primary', 'var(--brandPrimary, #3b82f6)'); + const rule = cssRule('cls', '.cls{--primary:transparent}'); + + const leafFirst = stylexPlugin.processStylexRules( + [blue500, brandPrimary, primary, rule], + { useLayers: false }, + ); + const aliasFirst = stylexPlugin.processStylexRules( + [primary, brandPrimary, blue500, rule], + { useLayers: false }, + ); + expect(leafFirst).toBe(aliasFirst); + expect(leafFirst).toMatchInlineSnapshot( + '".cls{--brandPrimary:transparent}"', + ); + }); + + test('rewrites each declaration in a multi-declaration rule one step', () => { + // Each declaration resolves one step (--primary -> --brandPrimary, + // --brandPrimary -> --blue500); a cascade would multi-hop --primary all + // the way to --blue500. + const consts = [ + constRule('primary', 'var(--brandPrimary, 0)'), + constRule('brandPrimary', 'var(--blue500, 0)'), + constRule('blue500', 'z'), + ]; + expect( + stylexPlugin.processStylexRules( + [...consts, cssRule('x', '.x{--primary:red;--brandPrimary:blue}')], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x{--brandPrimary:red;--blue500:blue}"'); + // same declarations, reversed text order -> each still resolves one step + expect( + stylexPlugin.processStylexRules( + [...consts, cssRule('x', '.x{--brandPrimary:blue;--primary:red}')], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x{--blue500:blue;--brandPrimary:red}"'); + }); + + test('substitutes a non-ASCII constant key', () => { + // `--`-prefixed keys are kept verbatim and may be non-ASCII; the scan must + // match them, not just an ASCII charset. + expect( + stylexPlugin.processStylexRules( + [constRule('café', 'red'), cssRule('x1', '.x1{color:var(--café)}')], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x1{color:red}"'); + }); + + test('does not resolve a non-ASCII alias chain (one step only)', () => { + // resolveConstant's pre-collapse regex is ASCII-only, so a non-ASCII + // const->const chain stops after one step (`--a` -> `var(--café)`), not + // through to `red`. Terminal non-ASCII keys still resolve (test above). + expect( + stylexPlugin.processStylexRules( + [ + constRule('a', 'var(--café)'), + constRule('café', 'red'), + cssRule('x1', '.x1{color:var(--a)}'), + ], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x1{color:var(--café)}"'); + }); + + test('leaves a rule with no constant references unchanged', () => { + expect( + stylexPlugin.processStylexRules( + [constRule('a', 'red'), cssRule('x1', '.x1{color:blue}')], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x1{color:blue}"'); + }); + + test('leaves rules unchanged when there are no constants', () => { + expect( + stylexPlugin.processStylexRules( + [cssRule('x1', '.x1{color:var(--a)}')], + { useLayers: false }, + ), + ).toMatchInlineSnapshot('".x1{color:var(--a)}"'); + }); + + test('substitutes a cross-file const end-to-end through transform', () => { + // defineConsts in its own .stylex module, consumed from a separate + // component module via a real `import`, as in a real app. + const tokensPath = '/src/app/tokens.stylex.js'; + const opts = { + parserOpts: { flow: 'all' }, + babelrc: false, + plugins: [ + [ + stylexPlugin, + { + unstable_moduleResolution: { + type: 'custom', + filePathResolver: () => tokensPath, + getCanonicalFilePath: (p) => p, + }, + }, + ], + ], + }; + const tokens = transformSync( + `import * as stylex from '@stylexjs/stylex'; + export const bp = stylex.defineConsts({ small: '@media (max-width: 600px)' });`, + { filename: tokensPath, ...opts }, + ); + const app = transformSync( + `import * as stylex from '@stylexjs/stylex'; + import { bp } from './tokens.stylex'; + export const styles = stylex.create({ + box: { color: { default: 'red', [bp.small]: 'blue' } }, + }); + export const out = stylex.props(styles.box);`, + { filename: '/src/app/App.js', ...opts }, + ); + const metadata = [ + ...(tokens.metadata.stylex || []), + ...(app.metadata.stylex || []), + ]; + // The const rule for `small` plus a `var(--…){…color:blue}` placeholder + // rule -- processStylexRules drops the @media string into the selector. + expect(metadata).toMatchInlineSnapshot(` + [ + [ + "xy5rbqd", + { + "constKey": "xy5rbqd", + "constVal": "@media (max-width: 600px)", + "ltr": "", + "rtl": null, + }, + 0, + ], + [ + "x1e2nbdu", + { + "ltr": ".x1e2nbdu{color:red}", + "rtl": null, + }, + 3000, + ], + [ + "xcoj4fz", + { + "ltr": "var(--xy5rbqd){.xcoj4fz.xcoj4fz{color:blue}}", + "rtl": null, + }, + 6000, + ], + ] + `); + expect(stylexPlugin.processStylexRules(metadata, { useLayers: false })) + .toMatchInlineSnapshot(` + ".x1e2nbdu{color:red} + @media (max-width: 600px){.xcoj4fz.xcoj4fz:not(#\\#){color:blue}}" + `); + }); + }); }); diff --git a/packages/@stylexjs/babel-plugin/src/index.js b/packages/@stylexjs/babel-plugin/src/index.js index c49e06257..711c4cc80 100644 --- a/packages/@stylexjs/babel-plugin/src/index.js +++ b/packages/@stylexjs/babel-plugin/src/index.js @@ -608,36 +608,60 @@ function processStylexRules( ); let lastKPri = -1; + // A `--` custom-property name up to the first unescaped CSS delimiter, + // used by both regexes below. Broader than resolveConstant's ASCII regex, so + // it matches non-ASCII keys -- but resolveConstant doesn't pre-collapse + // non-ASCII alias chains, so those resolve only one step here. + const CUSTOM_PROPERTY_KEY = '--[^\\s,:;)(}{\'"]+'; + // `var(--key)` value usages and `--key:` override declarations, both built + // from CUSTOM_PROPERTY_KEY. `var(--key)` won't match `var(--key, fallback)` + // -- those stay overridable, so we leave them. + const varUsageRegex = new RegExp(`var\\(${CUSTOM_PROPERTY_KEY}\\)`, 'g'); + const overrideKeyRegex = new RegExp(`(${CUSTOM_PROPERTY_KEY}):`, 'g'); + const hasConsts = constsMap.size > 0; const grouped = sortedRules.reduce((acc: Array>, rule) => { const [key, { ...styleObj }, priority] = rule; const priorityLevel = Math.floor(priority / 1000); - Object.keys(styleObj).forEach((dir) => { - let original = styleObj[dir]; - - for (const [varRef, constValue] of constsMap.entries()) { - if (typeof original !== 'string') continue; - - const replacement = String(constValue); - - original = original.replaceAll(varRef, replacement); - - // When the replacement is a variable, we need to replace the key to allow variable overrides - if (replacement.startsWith('var(') && replacement.endsWith(')')) { - const inside = replacement.slice(4, -1).trim(); - // Account for fallback variables - const commaIdx = inside.indexOf(','); - const targetName = ( - commaIdx >= 0 ? inside.slice(0, commaIdx) : inside - ).trim(); - - const constName = varRef.slice(4, -1); - original = original.replaceAll(`${constName}:`, `${targetName}:`); + if (hasConsts) { + Object.keys(styleObj).forEach((dir) => { + let original = styleObj[dir]; + if (typeof original !== 'string' || !original.includes('--')) { + return; } + // Pass 1: substitute `var(--key)` usages. One pass is enough -- + // resolveConstant pre-collapsed the alias chains, so values are terminal. + original = original.replace(varUsageRegex, (match) => { + const constValue = constsMap.get(match); + return constValue == null ? match : String(constValue); + }); + + // Pass 2 -- rewrite `--key:` override declarations (#1219): a single + // `replace` (not cumulative replaceAll) so one declaration's rewrite + // can't cascade into another's; each resolves one independent step. + original = original.replace(overrideKeyRegex, (match, cssVar) => { + const constValue = constsMap.get(`var(${cssVar})`); + if (constValue == null) { + return match; + } + const replacement = String(constValue); + // When the replacement is a variable, we need to replace the key to allow variable overrides + if (replacement.startsWith('var(') && replacement.endsWith(')')) { + const inside = replacement.slice(4, -1).trim(); + // Account for fallback variables + const commaIdx = inside.indexOf(','); + const targetName = ( + commaIdx >= 0 ? inside.slice(0, commaIdx) : inside + ).trim(); + return `${targetName}:`; + } + return match; + }); + styleObj[dir] = original; - } - }); + }); + } if (priorityLevel === lastKPri) { acc[acc.length - 1].push([key, styleObj, priority]);