Skip to content

[babel-plugin] Keep custom property rules inside @layer with useLayers #1752

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
ryanda9910 wants to merge 1 commit into
base: main
Choose a base branch
from
+107 −42
Open

[babel-plugin] Keep custom property rules inside @layer with useLayers #1752

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
47 changes: 47 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 @@ -350,6 +350,7 @@ describe('@stylexjs/babel-plugin', () => {
@layer priority1, priority2, priority3, priority4;
@property --x-color { syntax: "*"; inherits: false;}
@keyframes x35atj5-B{0%{box-shadow:1px 2px 3px 4px red;color:yellow;}100%{box-shadow:10px 20px 30px 40px green;color:var(--orange-theme-color);}}
@layer priority1{
:root, .xsg933n{--blue-xpqh4lw:blue;--marginTokens-x8nt2k2:10px;--colorTokens-xkxfyv:red;}
:root, .xbiwvf9{--small-x19twipt:2px;--medium-xypjos2:4px;--large-x1ec7iuc:8px;}
@media (prefers-color-scheme: dark){:root, .xsg933n{--colorTokens-xkxfyv:lightblue;}}
Expand All @@ -358,6 +359,7 @@ describe('@stylexjs/babel-plugin', () => {
.x6xqkwy.x6xqkwy, .x6xqkwy.x6xqkwy:root{--blue-xpqh4lw:lightblue;}
.x57uvma.x57uvma, .x57uvma.x57uvma:root{--large-x1ec7iuc:20px;--medium-xypjos2:10px;--small-x19twipt:5px;}
.--orange-theme-color-xufgesz{--orange-theme-color:red}
}
@layer priority2{
.margin-xymmreb{margin:10px 20px}
.padding-xss17vw{padding:var(--large-x1ec7iuc)}
Expand Down Expand Up @@ -385,6 +387,37 @@ describe('@stylexjs/babel-plugin', () => {
`);
});

// Regression test for https://github.com/facebook/stylex/issues/1611
// A `priorityLevel` (Math.floor(priority / 1000)) groups together CSS
// custom properties (priority 1) with priority-0 at-rules such as
// `@keyframes`. The layer-wrapping decision must not let the at-rule's
// priority 0 pull the layerable custom property out of its `@layer`.
test('custom properties stay inside @layer alongside priority-0 at-rules', () => {
const { metadata } = transform(`
import * as stylex from '@stylexjs/stylex';
const fade = stylex.keyframes({ from: { opacity: 0 }, to: { opacity: 1 } });
const styles = stylex.create({
card: {
animationName: fade,
'--card-padding': '16px',
color: 'red',
},
});
`);
const css = stylexPlugin.processStylexRules(metadata, {
useLayers: true,
enableLTRRTLComments: false,
});
// The @keyframes at-rule must stay unlayered (some CSS validators do not
// understand @keyframes/@property/@position-try inside @layer, #1071)...
expect(css).toMatch(/^@keyframes /m);
expect(css).not.toMatch(/@layer priority\d+\{[^}]*@keyframes/);
// ...but the custom property must be wrapped in its layer so external
// CSS layers can override it.
expect(css).toMatch(/@layer priority1\{[^]*--card-padding:16px[^]*\n\}/);
expect(css).not.toMatch(/\n\.[a-zA-Z0-9_-]+\{--card-padding:16px\}\n@/);
});

test('useLayers with before option', () => {
const { metadata } = transform(fixture);
expect(
Expand All @@ -399,6 +432,7 @@ describe('@stylexjs/babel-plugin', () => {
@layer reset, typography, priority1, priority2, priority3, priority4;
@property --x-color { syntax: "*"; inherits: false;}
@keyframes x35atj5-B{0%{box-shadow:1px 2px 3px 4px red;color:yellow;}100%{box-shadow:10px 20px 30px 40px green;color:var(--orange-theme-color);}}
@layer priority1{
:root, .xsg933n{--blue-xpqh4lw:blue;--marginTokens-x8nt2k2:10px;--colorTokens-xkxfyv:red;}
:root, .xbiwvf9{--small-x19twipt:2px;--medium-xypjos2:4px;--large-x1ec7iuc:8px;}
@media (prefers-color-scheme: dark){:root, .xsg933n{--colorTokens-xkxfyv:lightblue;}}
Expand All @@ -407,6 +441,7 @@ describe('@stylexjs/babel-plugin', () => {
.x6xqkwy.x6xqkwy, .x6xqkwy.x6xqkwy:root{--blue-xpqh4lw:lightblue;}
.x57uvma.x57uvma, .x57uvma.x57uvma:root{--large-x1ec7iuc:20px;--medium-xypjos2:10px;--small-x19twipt:5px;}
.--orange-theme-color-xufgesz{--orange-theme-color:red}
}
@layer priority2{
.margin-xymmreb{margin:10px 20px}
.padding-xss17vw{padding:var(--large-x1ec7iuc)}
Expand Down Expand Up @@ -448,6 +483,7 @@ describe('@stylexjs/babel-plugin', () => {
@layer priority1, priority2, priority3, priority4, overrides, xds.theme;
@property --x-color { syntax: "*"; inherits: false;}
@keyframes x35atj5-B{0%{box-shadow:1px 2px 3px 4px red;color:yellow;}100%{box-shadow:10px 20px 30px 40px green;color:var(--orange-theme-color);}}
@layer priority1{
:root, .xsg933n{--blue-xpqh4lw:blue;--marginTokens-x8nt2k2:10px;--colorTokens-xkxfyv:red;}
:root, .xbiwvf9{--small-x19twipt:2px;--medium-xypjos2:4px;--large-x1ec7iuc:8px;}
@media (prefers-color-scheme: dark){:root, .xsg933n{--colorTokens-xkxfyv:lightblue;}}
Expand All @@ -456,6 +492,7 @@ describe('@stylexjs/babel-plugin', () => {
.x6xqkwy.x6xqkwy, .x6xqkwy.x6xqkwy:root{--blue-xpqh4lw:lightblue;}
.x57uvma.x57uvma, .x57uvma.x57uvma:root{--large-x1ec7iuc:20px;--medium-xypjos2:10px;--small-x19twipt:5px;}
.--orange-theme-color-xufgesz{--orange-theme-color:red}
}
@layer priority2{
.margin-xymmreb{margin:10px 20px}
.padding-xss17vw{padding:var(--large-x1ec7iuc)}
Expand Down Expand Up @@ -498,6 +535,7 @@ describe('@stylexjs/babel-plugin', () => {
@layer reset, priority1, priority2, priority3, priority4, xds.theme;
@property --x-color { syntax: "*"; inherits: false;}
@keyframes x35atj5-B{0%{box-shadow:1px 2px 3px 4px red;color:yellow;}100%{box-shadow:10px 20px 30px 40px green;color:var(--orange-theme-color);}}
@layer priority1{
:root, .xsg933n{--blue-xpqh4lw:blue;--marginTokens-x8nt2k2:10px;--colorTokens-xkxfyv:red;}
:root, .xbiwvf9{--small-x19twipt:2px;--medium-xypjos2:4px;--large-x1ec7iuc:8px;}
@media (prefers-color-scheme: dark){:root, .xsg933n{--colorTokens-xkxfyv:lightblue;}}
Expand All @@ -506,6 +544,7 @@ describe('@stylexjs/babel-plugin', () => {
.x6xqkwy.x6xqkwy, .x6xqkwy.x6xqkwy:root{--blue-xpqh4lw:lightblue;}
.x57uvma.x57uvma, .x57uvma.x57uvma:root{--large-x1ec7iuc:20px;--medium-xypjos2:10px;--small-x19twipt:5px;}
.--orange-theme-color-xufgesz{--orange-theme-color:red}
}
@layer priority2{
.margin-xymmreb{margin:10px 20px}
.padding-xss17vw{padding:var(--large-x1ec7iuc)}
Expand Down Expand Up @@ -547,6 +586,7 @@ describe('@stylexjs/babel-plugin', () => {
@layer stylex.priority1, stylex.priority2, stylex.priority3, stylex.priority4;
@property --x-color { syntax: "*"; inherits: false;}
@keyframes x35atj5-B{0%{box-shadow:1px 2px 3px 4px red;color:yellow;}100%{box-shadow:10px 20px 30px 40px green;color:var(--orange-theme-color);}}
@layer stylex.priority1{
:root, .xsg933n{--blue-xpqh4lw:blue;--marginTokens-x8nt2k2:10px;--colorTokens-xkxfyv:red;}
:root, .xbiwvf9{--small-x19twipt:2px;--medium-xypjos2:4px;--large-x1ec7iuc:8px;}
@media (prefers-color-scheme: dark){:root, .xsg933n{--colorTokens-xkxfyv:lightblue;}}
Expand All @@ -555,6 +595,7 @@ describe('@stylexjs/babel-plugin', () => {
.x6xqkwy.x6xqkwy, .x6xqkwy.x6xqkwy:root{--blue-xpqh4lw:lightblue;}
.x57uvma.x57uvma, .x57uvma.x57uvma:root{--large-x1ec7iuc:20px;--medium-xypjos2:10px;--small-x19twipt:5px;}
.--orange-theme-color-xufgesz{--orange-theme-color:red}
}
@layer stylex.priority2{
.margin-xymmreb{margin:10px 20px}
.padding-xss17vw{padding:var(--large-x1ec7iuc)}
Expand Down Expand Up @@ -598,6 +639,7 @@ describe('@stylexjs/babel-plugin', () => {
@layer reset, typography, stylex.priority1, stylex.priority2, stylex.priority3, stylex.priority4, xds.theme;
@property --x-color { syntax: "*"; inherits: false;}
@keyframes x35atj5-B{0%{box-shadow:1px 2px 3px 4px red;color:yellow;}100%{box-shadow:10px 20px 30px 40px green;color:var(--orange-theme-color);}}
@layer stylex.priority1{
:root, .xsg933n{--blue-xpqh4lw:blue;--marginTokens-x8nt2k2:10px;--colorTokens-xkxfyv:red;}
:root, .xbiwvf9{--small-x19twipt:2px;--medium-xypjos2:4px;--large-x1ec7iuc:8px;}
@media (prefers-color-scheme: dark){:root, .xsg933n{--colorTokens-xkxfyv:lightblue;}}
Expand All @@ -606,6 +648,7 @@ describe('@stylexjs/babel-plugin', () => {
.x6xqkwy.x6xqkwy, .x6xqkwy.x6xqkwy:root{--blue-xpqh4lw:lightblue;}
.x57uvma.x57uvma, .x57uvma.x57uvma:root{--large-x1ec7iuc:20px;--medium-xypjos2:10px;--small-x19twipt:5px;}
.--orange-theme-color-xufgesz{--orange-theme-color:red}
}
@layer stylex.priority2{
.margin-xymmreb{margin:10px 20px}
.padding-xss17vw{padding:var(--large-x1ec7iuc)}
Expand Down Expand Up @@ -649,6 +692,7 @@ describe('@stylexjs/babel-plugin', () => {
@layer xds.reset, xds.typography, xds.base.priority1, xds.base.priority2, xds.base.priority3, xds.base.priority4, xds.theme;
@property --x-color { syntax: "*"; inherits: false;}
@keyframes x35atj5-B{0%{box-shadow:1px 2px 3px 4px red;color:yellow;}100%{box-shadow:10px 20px 30px 40px green;color:var(--orange-theme-color);}}
@layer xds.base.priority1{
:root, .xsg933n{--blue-xpqh4lw:blue;--marginTokens-x8nt2k2:10px;--colorTokens-xkxfyv:red;}
:root, .xbiwvf9{--small-x19twipt:2px;--medium-xypjos2:4px;--large-x1ec7iuc:8px;}
@media (prefers-color-scheme: dark){:root, .xsg933n{--colorTokens-xkxfyv:lightblue;}}
Expand All @@ -657,6 +701,7 @@ describe('@stylexjs/babel-plugin', () => {
.x6xqkwy.x6xqkwy, .x6xqkwy.x6xqkwy:root{--blue-xpqh4lw:lightblue;}
.x57uvma.x57uvma, .x57uvma.x57uvma:root{--large-x1ec7iuc:20px;--medium-xypjos2:10px;--small-x19twipt:5px;}
.--orange-theme-color-xufgesz{--orange-theme-color:red}
}
@layer xds.base.priority2{
.margin-xymmreb{margin:10px 20px}
.padding-xss17vw{padding:var(--large-x1ec7iuc)}
Expand Down Expand Up @@ -699,6 +744,7 @@ describe('@stylexjs/babel-plugin', () => {
@layer priority1, priority2, priority3, priority4;
@property --x-color { syntax: "*"; inherits: false;}
@keyframes x35atj5-B{0%{box-shadow:1px 2px 3px 4px red;color:yellow;}100%{box-shadow:10px 20px 30px 40px green;color:var(--orange-theme-color);}}
@layer priority1{
:root, .xsg933n{--blue-xpqh4lw:blue;--marginTokens-x8nt2k2:10px;--colorTokens-xkxfyv:red;}
:root, .xbiwvf9{--small-x19twipt:2px;--medium-xypjos2:4px;--large-x1ec7iuc:8px;}
@media (prefers-color-scheme: dark){:root, .xsg933n{--colorTokens-xkxfyv:lightblue;}}
Expand All @@ -707,6 +753,7 @@ describe('@stylexjs/babel-plugin', () => {
.x6xqkwy.x6xqkwy, .x6xqkwy.x6xqkwy:root{--blue-xpqh4lw:lightblue;}
.x57uvma.x57uvma, .x57uvma.x57uvma:root{--large-x1ec7iuc:20px;--medium-xypjos2:10px;--small-x19twipt:5px;}
.--orange-theme-color-xufgesz{--orange-theme-color:red}
}
@layer priority2{
.margin-xymmreb{margin:10px 20px}
.padding-xss17vw{padding:var(--large-x1ec7iuc)}
Expand Down
102 changes: 60 additions & 42 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 @@ -666,55 +666,73 @@ function processStylexRules(
';\n'
: '';

const collectedCSS = grouped
.map((group, index) => {
const pri = group[0][2];
const collectedCSS = Array.from(
new Map(group.map(([a, b]) => [a, b])).values(),
)
.flatMap((rule) => {
const { ltr, rtl } = rule;
let ltrRule = ltr,
rtlRule = rtl;

if (!useLayers && !legacyDisableLayers) {
ltrRule = addSpecificityLevel(ltrRule, index);
rtlRule = rtlRule && addSpecificityLevel(rtlRule, index);
}
const collectRules = (rules: Array<Rule>, index: number): string =>
Array.from(new Map(rules.map(([a, b]) => [a, b])).values())
.flatMap((rule) => {
const { ltr, rtl } = rule;
let ltrRule = ltr,
rtlRule = rtl;

if (!useLayers && !legacyDisableLayers) {
ltrRule = addSpecificityLevel(ltrRule, index);
rtlRule = rtlRule && addSpecificityLevel(rtlRule, index);
}

// check if the selector looks like .xtrlmmh, .xtrlmmh:root
// if so, turn it into .xtrlmmh.xtrlmmh, .xtrlmmh.xtrlmmh:root
// This is to ensure the themes always have precedence over the
// default variable values
ltrRule = ltrRule.replace(
// check if the selector looks like .xtrlmmh, .xtrlmmh:root
// if so, turn it into .xtrlmmh.xtrlmmh, .xtrlmmh.xtrlmmh:root
// This is to ensure the themes always have precedence over the
// default variable values
ltrRule = ltrRule.replace(
/\.([a-zA-Z0-9]+), \.([a-zA-Z0-9]+):root/g,
'.$1.$1, .$1.$1:root',
);
if (rtlRule) {
rtlRule = rtlRule.replace(
/\.([a-zA-Z0-9]+), \.([a-zA-Z0-9]+):root/g,
'.$1.$1, .$1.$1:root',
);
if (rtlRule) {
rtlRule = rtlRule.replace(
/\.([a-zA-Z0-9]+), \.([a-zA-Z0-9]+):root/g,
'.$1.$1, .$1.$1:root',
);
}
}

return rtlRule
? enableLTRRTLComments
? [
`/* @ltr begin */${ltrRule}/* @ltr end */`,
`/* @rtl begin */${rtlRule}/* @rtl end */`,
]
: [
addAncestorSelector(ltrRule, "html:not([dir='rtl'])"),
addAncestorSelector(rtlRule, "html[dir='rtl']"),
]
: [ltrRule];
})
.join('\n');
return rtlRule
? enableLTRRTLComments
? [
`/* @ltr begin */${ltrRule}/* @ltr end */`,
`/* @rtl begin */${rtlRule}/* @rtl end */`,
]
: [
addAncestorSelector(ltrRule, "html:not([dir='rtl'])"),
addAncestorSelector(rtlRule, "html[dir='rtl']"),
]
: [ltrRule];
})
.join('\n');

const collectedCSS = grouped
.map((group, index) => {
// A `priorityLevel` (Math.floor(priority / 1000)) can mix rules that must
// stay outside layers (`@property`, `@keyframes`, `@position-try` are all
// priority 0) with rules that belong inside them (CSS custom properties
// are priority 1). Deciding based on `group[0][2]` alone would let the
// first rule's priority pull the whole group out of its layer, so we
// partition the group by whether each rule is layerable (priority > 0).
if (!useLayers) {
return collectRules(group, index);
}

const layerable = group.filter(([, , pri]) => pri > 0);
const unlayered = group.filter(([, , pri]) => pri === 0);

const parts = [];
if (unlayered.length > 0) {
parts.push(collectRules(unlayered, index));
}
// Don't put @property, @keyframe, @position-try in layers
return useLayers && pri > 0
? `@layer ${layerName(index)}{\n${collectedCSS}\n}`
: collectedCSS;
if (layerable.length > 0) {
parts.push(
`@layer ${layerName(index)}{\n${collectRules(layerable, index)}\n}`,
);
}
return parts.join('\n');
})
.join('\n');

Expand Down