Skip to content

[babel-plugin] only substitute referenced constants in processStylexRules #1700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
henryqdineen wants to merge 1 commit into
base: main
Choose a base branch
from
+344 −23
Open

[babel-plugin] only substitute referenced constants in processStylexRules #1700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
297 changes: 297 additions & 0 deletions packages/@stylexjs/babel-plugin/__tests__/transform-process-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}"
`);
});
});
});
70 changes: 47 additions & 23 deletions packages/@stylexjs/babel-plugin/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,36 +608,60 @@ function processStylexRules(
);

let lastKPri = -1;
// A `--<key>` 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<Array<Rule>>, 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]);
Expand Down
Loading