Skip to content

Commit 6430896

Browse files
aryanraj45aryanraj45
committed
fix(css): write what the hoist took out as the rules it took
Two things the hoist owed the prefixing. A copy is a rule of its own — one selector an engine cannot read takes the whole list with it — so the run leaves each where it wrote it rather than joining them into a list a target drops whole. And a rule holding rules of its own is not hoisted: those were resolved against the prelude they were written under, which every copy but one is not, so a copy would go out without them.
1 parent 5977ebc commit 6430896

4 files changed

Lines changed: 80 additions & 20 deletions

File tree

‎.changeset/018-css-hoist-prefixed-nested.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"webpack": patch
33
---
44

5-
Take a nested rule out of its parent where a vendor prefix rewrote it, which left nesting a target that reads none.
5+
Hoist a vendor-prefixed nested rule for a target that reads no nesting.

‎lib/css/syntax-parser.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14819,7 +14819,9 @@ let _nestingAllowed = true;
1481914819
* naming its parent with `&`. `owner` is the rule they are now written against,
1482014820
* which is the only one that drains them — siblings share it, and taking an
1482114821
* earlier sibling's would write it against the wrong parent.
14822-
* @type {{ prelude: string, rest: string, owner: Node | null }[]}
14822+
* `apart` marks one a prefix copy wrote, which is written as a rule of its own:
14823+
* a selector an engine cannot read takes the whole list with it.
14824+
* @type {{ prelude: string, rest: string, owner: Node | null, apart?: boolean }[]}
1482314825
*/
1482414826
const _pendingNested = [];
1482514827

‎lib/css/syntax-printer.js‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1994,6 +1994,20 @@ const _declarationsPrecedeRules = (path, rule) => {
19941994
return true;
19951995
};
19961996

1997+
/**
1998+
* Whether a rule holds rules of its own, which a prefix copy could not carry:
1999+
* each was resolved against the prelude it was written under, and a copy is
2000+
* written under another.
2001+
* @param {Node} node the rule
2002+
* @returns {boolean} true when a rule is still waiting to be written in it
2003+
*/
2004+
const _holdsNested = (node) => {
2005+
for (let i = 0; i < _pendingNested.length; i++) {
2006+
if (_pendingNested[i].owner === node) return true;
2007+
}
2008+
return false;
2009+
};
2010+
19972011
/**
19982012
* The rules written in one rule, taken out of it: each drained entry's prelude
19992013
* is written against `prelude`, and where the rule is itself one written in
@@ -2009,7 +2023,7 @@ const _declarationsPrecedeRules = (path, rule) => {
20092023
const _writeNested = (path, node, prelude, rest, hoistable, copies) => {
20102024
// Only what was written in this rule: the walk is post-order, so an earlier
20112025
// sibling's rules are on the list too, waiting for the parent they share.
2012-
/** @type {{ prelude: string, rest: string, owner: Node | null }[]} */
2026+
/** @type {{ prelude: string, rest: string, owner: Node | null, apart?: boolean }[]} */
20132027
const taken = [];
20142028
for (let i = _pendingNested.length - 1; i >= 0; i--) {
20152029
if (_pendingNested[i].owner !== node) continue;
@@ -2026,10 +2040,17 @@ const _writeNested = (path, node, prelude, rest, hoistable, copies) => {
20262040
// written, rather than the whole group staying nested.
20272041
if (rest !== null && copies !== undefined) {
20282042
for (const copy of copies) {
2029-
_pendingNested.push({ prelude: copy, rest, owner });
2043+
_pendingNested.push({ prelude: copy, rest, owner, apart: true });
20302044
}
20312045
}
2032-
if (rest !== null) _pendingNested.push({ prelude, rest, owner });
2046+
if (rest !== null) {
2047+
_pendingNested.push({
2048+
prelude,
2049+
rest,
2050+
owner,
2051+
apart: copies !== undefined
2052+
});
2053+
}
20332054
for (let i = 0; i < taken.length; i++) {
20342055
taken[i].owner = owner;
20352056
_pendingNested.push(taken[i]);
@@ -2053,6 +2074,15 @@ const _writeNested = (path, node, prelude, rest, hoistable, copies) => {
20532074
}
20542075
for (let i = 0; i < taken.length; i++) {
20552076
const one = taken[i];
2077+
// A prefix copy is written as a rule of its own, since one selector an
2078+
// engine cannot read takes the whole list with it — so a list is not what
2079+
// these may be joined into.
2080+
if (one.apart === true) {
2081+
if (held !== null) out += _flushRun(held);
2082+
held = null;
2083+
out += _flushRun({ prelude: one.prelude, rest: one.rest, joined: false });
2084+
continue;
2085+
}
20562086
if (held !== null && held.rest === one.rest) {
20572087
// Concatenated, like every other join: the list is ordered once, below,
20582088
// where the run that grew it ends.
@@ -4648,11 +4678,13 @@ const printer = (path, writer) => {
46484678
}
46494679
);
46504680
}
4651-
// A rewritten rule is several rules with preludes of their own, which is
4652-
// no one block to join onto — but every copy carries this rule's block,
4653-
// so the hoist takes them out as the rules they are.
4681+
// A rewritten rule is several rules with preludes of their own, and each
4682+
// carries this rule's block — so the hoist takes them out. Only where
4683+
// it holds no rule of its own, those naming one prelude of the several.
46544684
const copies =
4655-
out === text || _prefixCopyPreludes.length === 0
4685+
out === text ||
4686+
_prefixCopyPreludes.length === 0 ||
4687+
_holdsNested(/** @type {Node} */ (_S._currentNode))
46564688
? undefined
46574689
: _prefixCopyPreludes;
46584690
const nested = _writeNested(

‎test/unitCases/CssSyntax.unittest.js‎

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11373,43 +11373,69 @@ describe("CssSyntax minify — nesting the target cannot read", () => {
1137311373
// the parent. A target that reads no nesting reads none of what stays.
1137411374
expect(
1137511375
minifyFor("a{color:red;&:autofill{color:blue}}", ["chrome 100"])
11376-
).toBe("a{color:red}a:-webkit-autofill,a:autofill{color:blue}");
11376+
).toBe("a{color:red}a:-webkit-autofill{color:blue}a:autofill{color:blue}");
1137711377
expect(
1137811378
minifyFor("a{color:red;&:autofill{color:blue}& b{color:teal}}", [
1137911379
"chrome 100"
1138011380
])
1138111381
).toBe(
11382-
"a{color:red}a:-webkit-autofill,a:autofill{color:blue}a b{color:teal}"
11382+
"a{color:red}a:-webkit-autofill{color:blue}a:autofill{color:blue}" +
11383+
"a b{color:teal}"
1138311384
);
1138411385
});
1138511386

11387+
it("writes what it took out the way a prefixed rule is written", () => {
11388+
// One selector an engine cannot read takes the whole list with it, which
11389+
// is why the prefixing wrote these as rules — the hoist keeps them so.
11390+
const T = ["chrome 100"];
11391+
expect(minifyFor("a{color:red;&:autofill{color:blue}}", T)).toBe(
11392+
`a{color:red}${minifyFor("a:autofill{color:blue}", T)}`
11393+
);
11394+
});
11395+
11396+
const HOIST_T = ["chrome 100", "firefox 100", "safari 15.4"];
11397+
1138611398
it.each([
11387-
["one prefixed rule", "a{&:fullscreen{top:0}}", "a:-webkit-full-screen,a:fullscreen{top:0}"],
11399+
[
11400+
"one prefixed rule",
11401+
"a{&:fullscreen{top:0}}",
11402+
"a:-webkit-full-screen{top:0}a:fullscreen{top:0}"
11403+
],
1138811404
[
1138911405
"a prefixed and a plain one",
1139011406
"a{&:defined{top:0}&:fullscreen{top:0}}",
11391-
"a:-webkit-full-screen,a:defined,a:fullscreen{top:0}"
11407+
"a:defined{top:0}a:-webkit-full-screen{top:0}a:fullscreen{top:0}"
1139211408
],
1139311409
[
1139411410
"two prefixed",
1139511411
"a{&:fullscreen{top:0}&:autofill{top:0}}",
11396-
"a:-webkit-autofill,a:-webkit-full-screen,a:autofill,a:fullscreen{top:0}"
11412+
"a:-webkit-full-screen{top:0}a:fullscreen{top:0}" +
11413+
"a:-webkit-autofill{top:0}a:autofill{top:0}"
1139711414
],
1139811415
[
1139911416
"a parent that declares as well",
1140011417
"a{top:0;&:fullscreen{left:0}}",
11401-
"a{top:0}a:-webkit-full-screen,a:fullscreen{left:0}"
11418+
"a{top:0}a:-webkit-full-screen{left:0}a:fullscreen{left:0}"
1140211419
]
1140311420
])("hoists %s", (_name, css, expected) => {
11404-
expect(minifyFor(css, ["chrome 100", "firefox 100", "safari 15.4"])).toBe(
11405-
expected
11406-
);
11421+
expect(minifyFor(css, HOIST_T)).toBe(expected);
1140711422
});
1140811423

1140911424
it("leaves a prefixed rule that was never nested alone", () => {
11425+
expect(minifyFor("a:fullscreen{top:0}", HOIST_T)).toBe(
11426+
"a:-webkit-full-screen{top:0}a:fullscreen{top:0}"
11427+
);
11428+
});
11429+
11430+
it("leaves a prefixed rule holding a rule of its own where it stands", () => {
11431+
// What it holds was resolved against the prelude it was written under,
11432+
// which every copy but one is not — so the group stays nested.
1141011433
expect(
11411-
minifyFor("a:fullscreen{top:0}", ["chrome 100", "firefox 100", "safari 15.4"])
11412-
).toBe("a:-webkit-full-screen{top:0}a:fullscreen{top:0}");
11434+
minifyFor("a{&:fullscreen{top:0;& .child{left:0}}}", HOIST_T)
11435+
).toBe(
11436+
"a{&:-webkit-full-screen{top:0}&:fullscreen{top:0}" +
11437+
"&:fullscreen .child{left:0}}"
11438+
);
1141311439
});
1141411440

1141511441
it("leaves a rule written in an at-rule where it stands", () => {

0 commit comments

Comments
 (0)