Skip to content

Commit 063d146

Browse files
aryanraj45aryanraj45
authored
fix(css): join the rules a nesting hoist leaves side by side (#22268)
* fix(css): join the rules a nesting hoist leaves side by side Lowering nesting writes a nested rule out on its own, as text, so the sibling merge never reads it: `a{.x{top:0}.y{top:0}}` printed two rules where a second pass writes `a .x,a .y{top:0}`. The run is joined where it is built, concatenating like every other join, and the list is ordered once where the run ends. The rule the hoist emptied stands in front of that run and takes the join too where it prints the same block. * fix(css): drop the guard the text cannot trip, and cover what the run reaches `text` is built from this rule's own `prelude` and `rest` a few lines above, so the check that they match could never fail — it did not fire once across 4232 parses of the comparison corpus. The run's cases are covered instead: a parent declaring nothing, a selector the hoist wrote twice, which the list reads back out, a deeper hoist feeding the run above it, and a parent written as a list. * fix(css): join the rule a hoist wrote onto the rules beside it A hoisted rule is written out as text, and the rule it was written in records nothing for it — so the writer read an opaque entry and offered it no join, whatever stood either side: `a{color:red}a{.x{color:red}}` printed two rules where a second pass writes `a,a .x{color:red}`. A run that wrote one rule records it, which is what a join reads a prelude off. A run that wrote several has no one prelude and stays as written. * chore(changeset): fold the two entries into one One changeset per pull request: both commits join what the hoist leaves beside each other, so they read as one line in the changelog.
1 parent a883efa commit 063d146

11 files changed

Lines changed: 353 additions & 12 deletions

File tree

‎.changeset/013-css-hoisted-nested-join.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack": patch
3+
---
4+
5+
Join the rules a lowered nesting hoist leaves beside each other, which only a second pass reached.

‎lib/css/syntax-parser.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15027,6 +15027,7 @@ module.exports._pendingNested = _pendingNested;
1502715027
module.exports._prefixRemovable = _prefixRemovable;
1502815028
module.exports._prefixScope = _prefixScope;
1502915029
module.exports._roundSignificant = _roundSignificant;
15030+
module.exports._canonicalSelectorList = _canonicalSelectorList;
1503015031
module.exports._ruleEntryOf = _ruleEntryOf;
1503115032
module.exports._rulePrelude = _rulePrelude;
1503215033
module.exports._setRuleEntry = _setRuleEntry;

‎lib/css/syntax-printer.js‎

Lines changed: 101 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ const {
162162
_appendChildTexts,
163163
_atKeyword,
164164
_blockSpans,
165+
_canonicalSelectorList,
165166
_collapseBox,
166167
_composeBlockBody,
167168
_customMedia,
@@ -1932,6 +1933,15 @@ const _resolveNestedPrelude = (prelude, parent) => {
19321933
return written.join(",");
19331934
};
19341935

1936+
// Whether the run `_writeNested` built took in the rule the hoist emptied,
1937+
// which is the caller's to write and so the caller's to leave out.
1938+
let _nestedTookParent = false;
1939+
// How many rules the run wrote, and the prelude length of the first. Read out
1940+
// of band, so a caller can say what its text is: a run that wrote one rule
1941+
// leaves a text that is one rule, which the rules around it may join onto.
1942+
let _nestedRuleCount = 0;
1943+
let _nestedFirstPrelude = 0;
1944+
19351945
/**
19361946
* Whether a block's declarations all stand before its rules, so taking the
19371947
* rules out of it leaves the cascade as it was.
@@ -1992,13 +2002,82 @@ const _writeNested = (path, node, prelude, rest, hoistable) => {
19922002
}
19932003
return null;
19942004
}
2005+
// Hoisting writes these out as text, so the sibling merge never reads them —
2006+
// two nested rules printing one block would stay two until a second pass.
2007+
// They are neighbors here and nowhere else, so the run is joined here.
19952008
let out = "";
2009+
_nestedRuleCount = 0;
2010+
_nestedFirstPrelude = 0;
2011+
/** @type {{ prelude: string, rest: string, joined: boolean } | null} */
2012+
let held = null;
2013+
// The rule the hoist emptied stands right in front of the run and may print
2014+
// the same block, so it is offered the join first. Read out of band, the
2015+
// caller writing that rule rather than this.
2016+
_nestedTookParent =
2017+
rest !== null && taken.length !== 0 && taken[0].rest === rest;
2018+
if (rest !== null && _nestedTookParent) {
2019+
held = { prelude, rest, joined: false };
2020+
}
19962021
for (let i = 0; i < taken.length; i++) {
1997-
out += taken[i].prelude + taken[i].rest;
2022+
const one = taken[i];
2023+
if (held !== null && held.rest === one.rest) {
2024+
// Concatenated, like every other join: the list is ordered once, below,
2025+
// where the run that grew it ends.
2026+
held.prelude += `,${one.prelude}`;
2027+
held.joined = true;
2028+
continue;
2029+
}
2030+
if (held !== null) out += _flushRun(held);
2031+
held = { prelude: one.prelude, rest: one.rest, joined: false };
19982032
}
2033+
if (held !== null) out += _flushRun(held);
19992034
return out;
20002035
};
20012036

2037+
/**
2038+
* Write one run out, counting it and taking down where the first one's block
2039+
* begins.
2040+
* @param {{ prelude: string, rest: string, joined: boolean }} run what the join left
2041+
* @returns {string} the rule
2042+
*/
2043+
const _flushRun = (run) => {
2044+
const prelude = _settledPrelude(run);
2045+
if (_nestedRuleCount === 0) _nestedFirstPrelude = prelude.length;
2046+
_nestedRuleCount++;
2047+
return prelude + run.rest;
2048+
};
2049+
2050+
/**
2051+
* Record what a hoisted run wrote, where it wrote one rule: a rule the writer
2052+
* can read a prelude off is one the rules either side of it may be joined onto,
2053+
* and a text that is several rules has no one prelude to read.
2054+
* @param {string} text what the run wrote
2055+
* @returns {void}
2056+
*/
2057+
const _recordHoistedRule = (text) => {
2058+
if (_nestedRuleCount !== 1) return;
2059+
_setRuleEntry(/** @type {Node} */ (_S._currentNode), {
2060+
text,
2061+
prelude: _nestedFirstPrelude,
2062+
atRule: false,
2063+
plain: true,
2064+
listable: LIST_UNKNOWN,
2065+
listKind: LIST_KIND_SELECTOR,
2066+
children: null,
2067+
head: "",
2068+
unordered: false
2069+
});
2070+
};
2071+
2072+
/**
2073+
* A joined run's selector list, canonical. A run that joined nothing is written
2074+
* as it stands, so a rule the hoist only moved pays no read of its own list.
2075+
* @param {{ prelude: string, rest: string, joined: boolean }} run what the join left
2076+
* @returns {string} the list to write
2077+
*/
2078+
const _settledPrelude = (run) =>
2079+
run.joined ? _canonicalSelectorList(run.prelude) : run.prelude;
2080+
20022081
/**
20032082
* Whether a rule's nested rules are taken out of it and written on their own.
20042083
* @param {CssPath} path the walk's accessors
@@ -4410,15 +4489,18 @@ const printer = (path, writer) => {
44104489
) {
44114490
_blockSpans.push(_NO_BLOCK_ENTRY);
44124491
// The rules written in it are not empty for its being so.
4413-
return (
4414-
_writeNested(
4415-
path,
4416-
/** @type {Node} */ (_S._currentNode),
4417-
prelude,
4418-
null,
4419-
true
4420-
) || ""
4421-
);
4492+
{
4493+
const written =
4494+
_writeNested(
4495+
path,
4496+
/** @type {Node} */ (_S._currentNode),
4497+
prelude,
4498+
null,
4499+
true
4500+
) || "";
4501+
if (minify && written.length !== 0) _recordHoistedRule(written);
4502+
return written;
4503+
}
44224504
}
44234505
// A rule that says which color scheme it is in says it again as the pair
44244506
// a lowered `light-dark()` reads, and — where it hands the choice to the
@@ -4507,7 +4589,15 @@ const printer = (path, writer) => {
45074589
);
45084590
if (nested === null) return "";
45094591
if (out !== text) return out + nested;
4510-
if (nested.length !== 0) return text + nested;
4592+
// The run wrote this rule's selectors into itself, so what it returns is
4593+
// the whole of both — `text` being this rule's own `prelude` and `rest`.
4594+
if (nested.length !== 0) {
4595+
if (_nestedTookParent) {
4596+
if (minify) _recordHoistedRule(nested);
4597+
return nested;
4598+
}
4599+
return text + nested;
4600+
}
45114601
// Where the block is what a sibling is compared against, remember the rule
45124602
// so the parent parts it without re-scanning for the `{`. An at-rule
45134603
// remembers its block's entries too, so a sibling's rules can join them.

‎test/configCases/css/minify-hoisted-nested-join/__snapshots__/ConfigCacheTest.snap‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
2+
3+
exports[`ConfigCacheTestCases css minify-hoisted-nested-join minify-hoisted-nested-join should compile 1`] = `".a .x,.a .y{top:0}.b .o,.b .p,.b .q{left:0}.c,.c .x,.c .y{right:0}.d{bottom:1px}.d .x,.d .y{bottom:0}.e .x{margin:0}.e .y{margin:1px}.e .z{margin:0}@media print{.f .x,.f .y{padding:0}}.g{top:0;.x{top:0}left:0}.h,.i .x{color:teal}.j .x,.k .y{outline-width:1px}.l,.m .x{outline-color:red}.n,.o,.o .x{order:3}"`;

‎test/configCases/css/minify-hoisted-nested-join/__snapshots__/ConfigTest.snap‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
2+
3+
exports[`ConfigTestCases css minify-hoisted-nested-join minify-hoisted-nested-join should compile 1`] = `".a .x,.a .y{top:0}.b .o,.b .p,.b .q{left:0}.c,.c .x,.c .y{right:0}.d{bottom:1px}.d .x,.d .y{bottom:0}.e .x{margin:0}.e .y{margin:1px}.e .z{margin:0}@media print{.f .x,.f .y{padding:0}}.g{top:0;.x{top:0}left:0}.h,.i .x{color:teal}.j .x,.k .y{outline-width:1px}.l,.m .x{outline-color:red}.n,.o,.o .x{order:3}"`;

‎test/configCases/css/minify-hoisted-nested-join/index.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import "./style.css";
2+
3+
it("should join the rules a hoist leaves side by side", () => {
4+
expect(true).toBe(true);
5+
});

‎test/configCases/css/minify-hoisted-nested-join/style.css‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/* Hoisting writes a nested rule out on its own, and two of them printing one
2+
block are neighbors nowhere else — so the join is offered there or left to
3+
a second pass. */
4+
.a { .x { top: 0 } .y { top: 0 } }
5+
6+
/* A run of them ends as one list, in the printer's own order. */
7+
.b { .q { left: 0 } .p { left: 0 } .o { left: 0 } }
8+
9+
/* The rule the hoist emptied stands in front of the run and prints the same
10+
block, so it joins too. */
11+
.c { right: 0; .x { right: 0 } .y { right: 0 } }
12+
13+
/* One that prints another block is left where it was written. */
14+
.d { bottom: 1px; .x { bottom: 0 } .y { bottom: 0 } }
15+
16+
/* A rule printing another block parts the run. */
17+
.e { .x { margin: 0 } .y { margin: 1px } .z { margin: 0 } }
18+
19+
/* Under the at-rule they were written in. */
20+
@media print { .f { .x { padding: 0 } .y { padding: 0 } } }
21+
22+
/* A declaration after a nested rule is read after it, so this block keeps its
23+
nesting and there is no run to join. */
24+
.g { top: 0; .x { top: 0 } left: 0 }
25+
26+
/* The hoist writes a rule out on its own, so what stands either side of it is
27+
its neighbor — and neighbors printing one block are one rule. */
28+
.h { color: teal }
29+
.i { .x { color: teal } }
30+
31+
/* Two hoists meeting, and a rule the hoist had nothing to do with. */
32+
.j { .x { outline-width: 1px } }
33+
.k { .y { outline-width: 1px } }
34+
.l { outline-color: red }
35+
.m { .x { outline-color: red } }
36+
37+
/* A run that took its own parent in is one rule too. */
38+
.n { order: 3 }
39+
.o { order: 3; .x { order: 3 } }

‎test/configCases/css/minify-hoisted-nested-join/test.config.js‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
"use strict";
2+
3+
const fs = require("fs");
4+
const path = require("path");
5+
const cssMinify = require("../../../../lib/css/cssMinify");
6+
7+
module.exports = {
8+
async afterExecute(options) {
9+
const css = fs.readFileSync(
10+
path.join(options.output.path, "bundle0.css"),
11+
"utf8"
12+
);
13+
expect(css).toMatchSnapshot();
14+
15+
expect(css).toContain(".a .x,.a .y{top:0}");
16+
expect(css).toContain(".b .o,.b .p,.b .q{left:0}");
17+
expect(css).toContain(".c,.c .x,.c .y{right:0}");
18+
expect(css).toContain(".d{bottom:1px}.d .x,.d .y{bottom:0}");
19+
expect(css).toContain(".e .x{margin:0}.e .y{margin:1px}.e .z{margin:0}");
20+
expect(css).toContain("@media print{.f .x,.f .y{padding:0}}");
21+
expect(css).toContain(".g{top:0;.x{top:0}left:0}");
22+
23+
// The rule a hoist wrote is a neighbor of what stands either side of it.
24+
expect(css).toContain(".h,.i .x{color:teal}");
25+
expect(css).toContain(".j .x,.k .y{outline-width:1px}");
26+
expect(css).toContain(".l,.m .x{outline-color:red}");
27+
expect(css).toContain(".n,.o,.o .x{order:3}");
28+
29+
// What the printer wrote is what a second pass writes.
30+
const again = await cssMinify({ "bundle0.css": css }, undefined, {
31+
environment: { browsers: ["chrome 100"] }
32+
});
33+
expect(again.code).toBe(css);
34+
}
35+
};

‎test/configCases/css/minify-hoisted-nested-join/webpack.config.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
"use strict";
2+
3+
/** @type {import("../../../../").Configuration} */
4+
module.exports = {
5+
// A target that reads no nesting is what hoists the nested rules out.
6+
target: "browserslist: chrome 100",
7+
mode: "production",
8+
output: {
9+
pathinfo: false
10+
},
11+
optimization: {
12+
minimize: true,
13+
minimizer: ["..."]
14+
},
15+
experiments: {
16+
css: true
17+
}
18+
};

‎test/external/wpt‎

Submodule wpt updated 1651 files

0 commit comments

Comments
 (0)