Visitar URL original
fix(css): fold no block of a layer over a streamed block of that layer by alexander-akait · Pull Request #22044 · webpack/webpack · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/023-css-layer-fold-past-streamed-block.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"webpack": patch
---

Keep a CSS layer's blocks in order where one of them streams.
25 changes: 25 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,31 @@ jobs:

- run: yarn link webpack --frozen-lockfile

# Keyed on the scripts that declare the package lists; a cache that
# predates a fixture being added is rebuilt by the install below, which
# reinstalls whenever the recorded list differs from the wanted one.
- name: Cache the comparison corpus
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: |
node_modules/.cache/css-tool-comparison
node_modules/.cache/html-tool-comparison
key: comparison-corpus-${{ runner.os }}-${{ hashFiles('tooling/compare-css-tools.js', 'tooling/compare-html-tools.js') }}

# The third corpus: the framework stylesheets and documents the tool
# comparisons install. Not fatal — the suite reads whichever fixtures are
# there and reports the tier as not built when the install did not land, so
# a registry outage costs that corpus rather than the job.
# One step each: `run` stops at the first command that fails, so a single
# block would cost both corpora where only one install went wrong.
- name: Install the CSS comparison corpus
continue-on-error: true
run: yarn benchmark:css-tools:setup

- name: Install the HTML comparison corpus
continue-on-error: true
run: yarn benchmark:html-tools:setup

- name: Cache jest result
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
Expand Down
7 changes: 7 additions & 0 deletions lib/css/syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -4990,6 +4990,13 @@ const _streamClose = (d, rule) => {
_frameSeenDeclarations[d] = null;
_frameSeenRules[d] = null;
_frameSeenLayers[d] = null;
// A streamed block is one its parent's gather never saw, so nothing folds
// back over it — it may write into a layer already noted there.
if (d === 0) {
if (_seenTopLevelLayers !== null) _seenTopLevelLayers.clear();
} else if (_frameSeenLayers[d - 1] !== null) {
/** @type {Map<string, SeenLayer>} */ (_frameSeenLayers[d - 1]).clear();
}
if (writer.isPending(_framePendingDepth[d])) {
// Nothing inside printed. An empty rule paints nothing, so dropping it
// leaves the cascade as it was — but only where the block itself carries no
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@
"build:examples": "cd examples && node buildAll.js",
"benchmark": "node --max-old-space-size=6144 --experimental-vm-modules --trace-deprecation --hash-seed=1 --random-seed=1 --no-opt --predictable --predictable-gc-schedule --interpreted-frames-native-stack --allow-natives-syntax --expose-gc --no-concurrent-sweeping ./test/BenchmarkTestCases.benchmark.mjs",
"benchmark:css-tools": "node tooling/compare-css-tools.js",
"benchmark:css-tools:setup": "node tooling/compare-css-tools.js --setup",
"benchmark:html-tools": "node tooling/compare-html-tools.js",
"benchmark:html-tools:setup": "node tooling/compare-html-tools.js --setup",
"benchmark:js-tools": "node tooling/compare-js-tools.js",
"measure:color-agreement": "node tooling/measure-color-agreement.js",
"pretest": "yarn lint",
Expand Down
11 changes: 11 additions & 0 deletions test/CssSyntax.unittest.js
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,17 @@ describe("CssSyntax — block streaming", () => {
expect(out.slice(at, at + victim.length)).toBe(victim);
});

it("folds no block of a layer over a streamed block of that layer", () => {
// A streamed block is one its parent's gather never saw, so a later block
// of its layer must not fold in front of it — that order is the cascade's.
const src = `@layer outer{@layer a{.one{color:red}}@layer a{${BIG}}@layer a{.three{color:teal}}}`;
// The middle block has to stream for the gather to be the one under test.
expect(childCount(`@layer a{${BIG}}`)).toBe(0);
const out = minify(src);
expect(out.indexOf(".one")).toBeLessThan(out.indexOf(".c0"));
expect(out.indexOf(".c0")).toBeLessThan(out.indexOf(".three"));
});

it("enters a streamed rule before its children and exits after them", () => {
const seq = walk(`@media screen{${SMALL}}`, { recurseBlocks: true });
expect(seq[0]).toBe("+AtRule|0|0");
Expand Down
132 changes: 116 additions & 16 deletions test/helpers/syntaxEquivalence.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@

const fs = require("fs");
const path = require("path");
// Read from the comparisons, so a fixture added to one is added here.
const {
CACHE: CSS_CACHE,
GENERATED_FIXTURES,
INSTALLED_FIXTURES
} = require("../../tooling/compare-css-tools");
const {
APP_SHELL,
CACHE: HTML_CACHE,
INLINED_STYLESHEETS,
INSTALLED_DOCUMENTS,
inlineCssPage
} = require("../../tooling/compare-html-tools");

/** @typedef {{ name: string, raw: string, min: string }} Fixture */

Expand Down Expand Up @@ -61,6 +74,76 @@ const buildCorpus = (dir, extension, minify) => {
});
};

/**
* Read a benchmark cache as a corpus. A file the cache does not hold is left
* out: each cache is built by the comparison that installs it.
* @param {string} cache the cache directory
* @param {[string, string][]} entries `[label, path within the cache]`
* @param {(source: string) => string} minify the printer to run
* @returns {Fixture[]} the corpus, empty when the cache is not there
*/
const readBenchmarkCache = (cache, entries, minify) => {
/** @type {Fixture[]} */
const out = [];
for (const [label, within] of entries) {
const file = path.join(cache, within);
if (!fs.existsSync(file)) continue;
const raw = fs.readFileSync(file, "utf8");
out.push({ name: label, raw, min: minify(raw) });
}
return out;
};

/**
* The stylesheets `yarn benchmark:css-tools` installs and builds.
* @param {(source: string) => string} minify the printer to run
* @returns {Fixture[]} the corpus, empty until that has been run once
*/
const benchmarkStylesheets = (minify) =>
readBenchmarkCache(
CSS_CACHE,
[
...INSTALLED_FIXTURES.map(
(/** @type {[string, string]} */ [label, file]) =>
/** @type {[string, string]} */ ([label, `node_modules/${file}`])
),
...GENERATED_FIXTURES
],
minify
);

/**
* The documents `yarn benchmark:html-tools` installs, and the pages it builds
* around a whole framework stylesheet. Those pages are the ones carrying a
* `<style>`, which is what reaches the css minifier nested in the html one.
* @param {(source: string) => string} minify the printer to run
* @returns {Fixture[]} the corpus, empty until that has been run once
*/
const benchmarkDocuments = (minify) => {
const out = readBenchmarkCache(
HTML_CACHE,
INSTALLED_DOCUMENTS.map(
(/** @type {[string, string]} */ [label, file]) =>
/** @type {[string, string]} */ ([label, `node_modules/${file}`])
),
minify
);
if (out.length > 0) {
out.push({
name: "App shell (inline critical CSS)",
raw: APP_SHELL,
min: minify(APP_SHELL)
});
}
for (const [label, file] of INLINED_STYLESHEETS) {
const sheet = path.join(HTML_CACHE, "node_modules", file);
if (!fs.existsSync(sheet)) continue;
const raw = inlineCssPage(label, fs.readFileSync(sheet, "utf8"));
out.push({ name: label, raw, min: minify(raw) });
}
return out;
};

/**
* @typedef {{ kind: string, condition: string }} Condition
* @typedef {{ chain: Condition[], text: string, label?: string, list?: string[], block?: string }} Rule
Expand Down Expand Up @@ -493,9 +576,10 @@ const installHelpers = () => {
* substitution is compared as parsed because `var(--a)` and `var(--b)` both
* compute to nothing on a probe with no ancestor to resolve them.
* @param {string} declaration the declaration block
* @param {CSSStyleDeclaration=} own the block's own declarations, when it has
* them: Chrome serializes an unterminated `var(--a` as `var(--a;`, which no
* longer re-parses, so the probe alone would lose it
* @param {CSSStyleDeclaration[]=} own the blocks' own declarations, later
* winning: a round trip through `cssText` is lossy, so the probe alone would
* lose an unterminated `var(--a` (serialized as `var(--a;`) and every
* longhand of a shorthand awaiting substitution (serialized with no value)
* @returns {string[]} one entry per property it sets, unordered
*/
const computed = (declaration, own) => {
Expand All @@ -505,15 +589,26 @@ const installHelpers = () => {
// the previous rule's value, and the computed style would be read in flight.
for (const animation of probe.getAnimations()) animation.cancel();
const style = getComputedStyle(probe);
const source = own || probe.style;
/** @type {string[]} */
const out = [];
/** @type {Map<string, [string, string]>} */
const stated = new Map();
// Indexed rather than iterated: a `@function` body's `result` descriptor is
// counted and named by `item()`, but Chrome's iterator hands back nothing.
for (let at = 0; at < source.length; at++) {
const property = source.item(at);
const specified = source.getPropertyValue(property);
const bang = source.getPropertyPriority(property) === "" ? "" : "!";
for (const source of own || [probe.style]) {
for (let at = 0; at < source.length; at++) {
const property = source.item(at);
// CSS Cascade 4 §6.2: an important declaration is not overridden by a
// normal one written after it, so the later block does not take it.
const priority = source.getPropertyPriority(property);
const earlier = stated.get(property);
if (earlier === undefined || priority !== "" || earlier[1] === "") {
stated.set(property, [source.getPropertyValue(property), priority]);
}
}
}
/** @type {string[]} */
const out = [];
for (const [property, [specified, priority]] of stated) {
const bang = priority === "" ? "" : "!";
// A custom property is a token stream the engine keeps verbatim, so it is
// compared as written too — the whitespace and comments between its
// tokens say nothing once it is substituted.
Expand Down Expand Up @@ -556,7 +651,7 @@ const installHelpers = () => {
/** @type {Rule[]} */
const out = [];
// What each selector has been told so far, under each chain of conditions.
/** @type {Map<string, string>} */
/** @type {Map<string, { css: string, styles: CSSStyleDeclaration[] }>} */
const carried = new Map();
// A nested declaration block is its own interface, so it can be recognized
// rather than guessed at from the shape of its text.
Expand Down Expand Up @@ -791,7 +886,7 @@ const installHelpers = () => {
// about that selector so far: a printer is free to move a selector
// between two adjacent lists, so only the per-selector sequence is
// the thing both sides have to agree on.
const own = computed(style.cssText, style);
const own = computed(style.cssText, [style]);
const block = ` { ${[...own].sort().join(";")} }`;
const where = held
.map((one) => `${one.kind}\u0001${conditionKey(one)}`)
Expand All @@ -802,12 +897,15 @@ const installHelpers = () => {
const css =
earlier === undefined
? style.cssText
: `${earlier};${style.cssText}`;
carried.set(key, css);
: `${earlier.css};${style.cssText}`;
const styles =
earlier === undefined ? [style] : [...earlier.styles, style];
carried.set(key, { css, styles });
// Read as one block, which is what the cascade reads: a
// percentage or a `min()` here resolves against the earlier
// declarations, so the two lists cannot simply be added.
const list = earlier === undefined ? own : computed(css);
// declarations, so the two lists cannot simply be added. The
// declarations themselves still say which properties are set.
const list = earlier === undefined ? own : computed(css, styles);
out.push({
chain: held,
label: one,
Expand Down Expand Up @@ -1727,6 +1825,8 @@ const compareRules = (before, after, signatures) => {
};

module.exports = {
benchmarkDocuments,
benchmarkStylesheets,
buildCorpus,
collectFixtures,
compareRules,
Expand Down
59 changes: 55 additions & 4 deletions test/syntaxEquivalence.spectest.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ const {
const expectNoDeprecations = require("./helpers/expectNoDeprecations");
const launchChrome = require("./helpers/launchChrome");
const {
benchmarkDocuments,
benchmarkStylesheets,
buildCorpus,
compareRules,
conditionSignatures,
Expand Down Expand Up @@ -74,6 +76,10 @@ const FILED_CONFIG_CSS_DEFECTS = new Map();

const FILED_CONFIG_HTML_DEFECTS = new Map();

const FILED_BENCHMARK_CSS_DEFECTS = new Map();

const FILED_BENCHMARK_HTML_DEFECTS = new Map();

const FILED_WPT_HTML_DEFECTS = new Map([
[
"test/wpt/html/syntax/parsing/misnested-form-in-template.html",
Expand Down Expand Up @@ -237,6 +243,21 @@ const buildCorpora = () => {
filedCss: FILED_CONFIG_CSS_DEFECTS
}
];
// Real projects, where `configCases` and wpt are both written to exercise a
// rule rather than to ship.
const benchHtml = benchmarkDocuments((source) => source);
const benchCss = benchmarkStylesheets(minifyCss);
if (benchHtml.length > 0 || benchCss.length > 0) {
built.push({
label: "benchmark corpus",
html: variant(benchHtml, {}),
htmlAllImpliedTags: variant(benchHtml, { removeImpliedTags: true }),
htmlSmartTags: variant(benchHtml, { removeImpliedTags: "smart" }),
css: benchCss,
filedHtml: FILED_BENCHMARK_HTML_DEFECTS,
filedCss: FILED_BENCHMARK_CSS_DEFECTS
});
}
if (!hasCorpus()) return built;
/** @type {Fixture[]} */
const wptHtml = [];
Expand Down Expand Up @@ -289,6 +310,9 @@ const inBatches = async (page, items, evaluate) => {
const NO_CORPUS =
"wpt submodule not initialized (run `git submodule update --init --depth 1 test/wpt`)";

const NO_BENCHMARK_CORPUS =
"comparison caches not built (run `yarn benchmark:css-tools` / `yarn benchmark:html-tools`)";

expectNoDeprecations();

describe("printer output in real Chrome", () => {
Expand Down Expand Up @@ -631,6 +655,28 @@ describe("printer output in real Chrome", () => {
FILE_TIMEOUT
);

// CSS Cascade 4 §6.2: a normal declaration written after an important one
// does not override it, so the later block is the dead one.
it(
"reads a repeated selector's importance as the cascade does",
async () => {
const differences = await compareStylesheets([
{
name: "important-then-normal",
raw: ".a{--x:red!important}.a{--x:blue}",
min: ".a{--x:red!important}"
},
{
name: "normal-then-normal",
raw: ".a{--y:red}.a{--y:blue}",
min: ".a{--y:blue}"
}
]);
expect(differences).toEqual([]);
},
FILE_TIMEOUT
);

// A value that is not itself a color still carries them, and the computed
// value keeps the space each was written in.
it(
Expand Down Expand Up @@ -695,10 +741,15 @@ describe("printer output in real Chrome", () => {
// Which corpora were built depends on what is checked out, so each names
// itself, and one that could not be built says so rather than going quiet.
for (const at of corpora.keys()) describeCorpus(at);
if (!corpora.some((one) => one.label === "wpt")) {
describe("wpt", () => {
it(NO_CORPUS, () => {
// No-op: the corpus is an optional git submodule.
for (const [label, why] of [
["wpt", NO_CORPUS],
["benchmark corpus", NO_BENCHMARK_CORPUS]
]) {
if (corpora.some((one) => one.label === label)) continue;

describe(label, () => {
it(why, () => {
// No-op: both are optional, and each is built outside this suite.
});
});
}
Expand Down
Loading
Loading