Skip to content

perf(css): recycle nested block contents during the walk - #21628

Merged
alexander-akait merged 11 commits into
mainfrom
perf/css-stream-nested-blocks
Aug 9, 2026
Merged

alexander-akait merged 11 commits into
mainfrom
perf/css-stream-nested-blocks

Conversation

@alexander-akait

@alexander-akait alexander-akait commented Aug 7, 2026 •

Copy link
Copy Markdown
Member

Summary

The CSS grammar streams and recycles per top-level node, so anything wrapped in one large block holds its whole subtree: 20k rules inside a single @media keep 8.05 MiB of columns, against 0 for the same rules written flat. This enters a rule once its block grows past a threshold, then walks and recycles each later child as it completes, so peak storage is the open path rather than the block. Draft: the memory goal is met on every shape, CPU is not yet at parity (see below).

Measured, 20k rules per shape, walk-only, interleaved min-of-N:

shape CPU memory before memory after
@media { …20k… } +1.8% 8.05 MiB 0
@supports { …20k… } +4.5% 8.05 MiB 0
@media{@supports{ … }} +4.1% 8.05 MiB 0
CSS nesting in one rule +1.9% 4.39 MiB 0
flat (each rule top-level) +7.4% 0 0
many small @media +7.7% 0 0

Streaming is opt-in (streamBlocks) and walk-only, so nothing in CssParser or the printer changes yet.

What kind of change does this PR introduce?

perf

Did you add tests for your changes?

Not yet — draft. Verified so far with a differential against the non-streaming walk (27 shapes including six past 6,000 rules): 0 sequence differences, every delta being the provisional end reported at enter, 0 at exit. Existing suites are unaffected because streaming is opt-in: 320/320 CssSyntax unit tests and 917/917 configCases/css.

Does this PR introduce a breaking change?

No — opt-in and off by default. Two semantic notes for when it is enabled: a rule entered before its block closes reports a provisional end at enter (final at exit), and a streamed rule hands its children to the visitors instead of collecting them, so A.declarations / A.childRules read empty on it.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

n/a — internal parser option, not part of the public config surface.

Use of AI

Claude Code was used to implement this change and to run the measurements and the differential harness reported above. All numbers in this description are from runs on this branch against main; the design decisions and the correctness checks were reviewed by a human before each commit.


Generated by Claude Code

Summary by CodeRabbit

  • Performance

    • Improved processing of large and deeply nested CSS blocks through incremental streaming.
    • Reduced memory usage during CSS parsing and printing while preserving source order.
  • Bug Fixes

    • Improved handling of empty rules, duplicate declarations, comments, minified output, and nested blocks.
    • Prevented shutdown-related crashes during Bun test execution.
  • Reliability

    • Improved source-map generation and processing stability.
    • Expanded coverage for traversal, skipped children, error recovery, and cleanup.

@changeset-bot

changeset-bot Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: aef7cf8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actions Bot commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor

This PR is packaged and the instant preview is available (ce12f4e).

Install it locally:

  • npm
npm i -D webpack@https://pkg.pr.new/webpack@ce12f4e
  • yarn
yarn add -D webpack@https://pkg.pr.new/webpack@ce12f4e
  • pnpm
pnpm add -D webpack@https://pkg.pr.new/webpack@ce12f4e

@codecov

codecov Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.41%. Comparing base (6412396) to head (aef7cf8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #21628      +/-   ##
==========================================
+ Coverage   94.39%   94.41%   +0.01%     
==========================================
  Files         628      628              
  Lines       80273    80611     +338     
  Branches    23732    23809      +77     
==========================================
+ Hits        75777    76107     +330     
- Misses       4496     4504       +8     
Flag Coverage Δ
css-parsing 25.86% <27.56%> (-0.03%) ⬇️
html5lib 30.25% <1.04%> (-0.04%) ⬇️
integration 88.59% <64.91%> (-0.16%) ⬇️
test262 49.09% <ø> (ø)
unit 52.45% <100.00%> (+0.23%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed

codspeed Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
❌ 1 regressed benchmark
✅ 217 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Memory benchmark "many-modules-esm", scenario '{"name":"mode-development","mode":"development"}' 821.2 KB 2,304.4 KB -64.36%
⚡ Memory unit benchmark "css-parser-tailwind-unit", process (expanded, no visitors) 4.7 MB 1.2 MB ×3.9
⚡ Memory benchmark "asset-modules-inline", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 397.6 KB 272.7 KB +45.82%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing perf/css-stream-nested-blocks (aef7cf8) with main (6412396)

Open in CodSpeed

@alexander-akait
alexander-akait force-pushed the perf/css-stream-nested-blocks branch from bdc662b to bf19f97 Compare August 7, 2026 11:02
@github-actions

github-actions Bot commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor

Generated code size

Comparing aef7cf8 against 6412396. Merging this PR will change the code webpack generates.

Changed New Deleted Unchanged Raw change
Cases 1 0 0 1509 🔴 ↑ +1 B
Assets 1 0 0 5906 🔴 ↑ +1 B
Runtimes 0 0 0 1859 —
1 asset(s) changed size
Asset Before After Change Gzip (9) Brotli (11) Zstd (19)
🔴 ↑ module/import-meta-parser-options bundle0.mjs 20.55 KiB 20.55 KiB +1 B (+0.00%) +0.11% +0.19% +0.11%

No runtime gained or lost a runtime module.

Built test/configCases with the defaults a user gets: 1510 case(s), 5907 asset(s), 35 emitted nothing.

The walk already streamed top-level nodes, but everything inside one block
was held until that whole subtree finished, so a stylesheet wrapped in a
single `@media` kept its entire AST. Enter a rule as soon as its prelude is
sealed at `{`, walk and recycle each child as it completes, and exit at `}`
where the rule's end is final — peak storage becomes the open path.

Off by default (`streamBlocks`) and walk-only. A streamed rule's children
reach the visitors instead of its body, so `A.declarations` / `A.childRules`
read empty for it; `CssParser` reads both and cannot opt in until those
sites accumulate during the walk instead.

Peak column storage over 20k rules: `@media`-wrapped 8.05 MiB -> 0, the same
for `@supports` and nested at-rules, CSS nesting 4.39 MiB -> 0. Shapes with
nothing to reclaim pay for the bookkeeping (flat +9%, many small blocks
+17%) — the lazy activation threshold that fixes it is not implemented yet.

Differential against the default walk over 20 shapes: no ordering or count
differences, and the whole delta is the provisional `end` a rule reports at
`enter` before its block closes.
`_streamConsumeBlock` recurses through the block consumer, so its own locals
already are the stack — the four parallel arrays it pushed and popped per
rule were eight array operations, with their bounds and capacity checks, on
every rule. It cost 5.2% self time in a CPU profile of a flat stylesheet,
where streaming reclaims nothing and the work is pure overhead.

Save the frame into locals across the recursive call instead. Interleaved
A/B, min of 35 samples per side, 20k rules: flat +9.3% -> +1.8%, many small
blocks +17.2% -> +10.6%, one big `@media` -23.9% -> -2.7%, `@supports`
+3.0% -> -1.7%, nested at-rules -5.2% -> -3.3%, CSS nesting +0.8% -> -0.6%.

Memory is unchanged: every shape that held a subtree still recycles to zero.
The remaining cost on small blocks is the streaming machinery running where
there is nothing to reclaim, which the lazy activation threshold addresses.
The streamed walk ran on every block, so a small one paid the machinery for
nothing: +9.3% on a flat stylesheet, +17.2% on many small `@media`.

Enter a rule and start recycling only once its block passes
`_STREAM_MIN_NODES`. `_nodeCount` restarts at each top-level node, so
`_nodeCount - mark` is exactly how far the current block has grown; under
the threshold the sink collects like the non-streaming path and the rule is
handed back materialized for the ordinary walk, keeping `A.declarations` /
`A.childRules` readable on it.

Activation has to enter every still-buffering ancestor first, outermost in,
or a nested block enters before its parent — `@media{@supports{...}}` fired
the inner rule's `enter` first. Ancestors are unreachable from locals, so
the per-block frame moved into depth-indexed tables: nesting is shallow and
strictly LIFO, so an indexed store avoids the bounds and capacity checks
that push/pop cost on every rule.

Memory is now released on every shape that used to hold a subtree: one big
`@media`, `@supports`, nested at-rules and CSS nesting all go from
8.05 / 4.39 MiB to 0. CPU is +1.8% to +7.7% — the sink still costs an
indirect call per child before activation, which the next commit removes by
checking the threshold inside the block consumer instead.
A block wrote its frame and pushed every child through the sink from the
moment it opened, so a stylesheet with no big block paid for machinery it
never used. Keep the block's state in `consumeABlocksContentsInto`'s locals
and write the frame only where an ancestor could still be reached for --
just before a child rule, and on the block's own growth past the threshold.
A block of nothing but declarations, which is every leaf rule, now touches
no frame at all, and the growth check is one compare in the collect loop
rather than a call and a type test per child.

Two fixes fall out of entering a rule before its block is consumed:

- Sibling indices were wrong on every streamed block: the parent's counter
  was bumped when a child block opened and then reused as the walk counter,
  so the first child of a large `@media` reported index 456. Each list now
  counts separately, as `_walkRule` does, and the index a child carries is
  the one the collected walk gives it.
- `A.declarations` / `A.childRules` returned `null` on a streamed rule --
  indistinguishable from an at-rule with no block, which is what
  `CssParser` reads to decide. The body slot is claimed, empty, before
  `enter` fires; claiming it up front also lets the slots be handed back
  per child, which is the last thing a streamed block still grew.

Threshold raised to 16384 nodes: a block that crosses it just before its
own `}` activates for almost nothing back.

Measured against main, 20k rules per shape, each arm in its own process,
median of 13: @media -9.6%, @supports -8.0%, @media>@supports -11.4%, CSS
nesting -7.5%, flat +0.2%, many small @media +1.0%. Retained heap while the
block is open, forced GC: 31.2 MiB of node columns and 3.9 MiB of JS heap
down to 0 and 0.16 MiB. The default path is unchanged.
`streamBlocks` was walk-only, because the printer builds a parent's text
out of its children's -- exactly what a streamed body has already released.
So printing held the whole subtree: 60 MiB live while a 20k-rule `@media`
was open, for 3.2 MiB of output.

A streamed rule now prints as it goes: its `prelude{` is held back when the
block opens, each finished child is emitted straight after it, and the `}`
closes it. Held rather than emitted, because an opener that turns out to
have nothing inside it still has to be droppable at its `}` -- a whole
stack of them, since the block that empties may be several levels down.

Two transforms genuinely need a whole block at once, and neither is given
up:

- A longhand family only merges in a block with no child rule, so a block
  of declarations alone is the one that declines to stream.
- Only the last of a set of identical declarations can be read. There is no
  looking ahead for the later one, so the earlier one is taken back out of
  the output instead: `PrintContext` keeps the output as the pieces it was
  emitted in, and a piece can be retracted after later ones are out.

Positions follow from that -- a piece's place in the output is no longer
known when it is emitted, so mappings anchor to a piece and are resolved
against the pieces once, when a map is asked for. Printing without one now
never scans the text for newlines at all.

Output is byte for byte the collected printer's, source map included, over
23 shapes covering empty blocks, duplicate declarations across a child
rule, kept comments, raw hacks, custom properties and `@keyframes`.

Minifying 20k rules, each arm in its own process, median of 11: `@media`
-36.8%, `@media>@supports` -38.1%, CSS nesting -36.9%, flat +0.4%, many
small `@media` +2.0%. Retained heap while the block is open: 60.4 -> 7.7
MiB on `@media`, 16.3 -> 3.8 MiB on nesting, against 3.2 and 0.7 MiB of
output. The collected path is unchanged, on both CSS and HTML.
…here needed

Two regressions on stylesheets that never stream, both from how the output
is now held.

Every emit became a piece of its own, so a flat stylesheet -- the common
one -- built an array of 20k strings where it used to append to one. Output
accumulates into a plain string again, and a piece is cut off only for text
that may still be taken back, which is the block's own declarations and
nothing else. Mappings carry an offset within their piece, so an anchor can
sit mid-string.

The other was subtler and was costing 3.8 MiB. A printer returns its text
as a rope of its children's pieces, and the old `_emit` happened to flatten
each one while scanning it for newlines -- a scan this branch had removed as
useless, since positions are now worked out only if a map is asked for.
Without it, ropes append onto ropes and every fragment of every node stays
reachable until the output is finally flattened: a second copy of the whole
output. Flattening each piece as it lands fixes the memory and costs 40% CPU,
an allocation per piece. So the tail is cut off and forced flat once per 64
KiB instead, which the result would have copied anyway.

Measured against main, each arm in its own process, arms alternated:

  minify, retained heap while the block is open, 20k rules
    @media      60.4 -> 3.4 MiB    flat   5.9 -> 5.0 MiB
    nesting     16.3 -> 1.2 MiB    small @media  3.9 -> 3.9 MiB
  minify CPU, 10k rules, median of 9
    @media -35.3%   @supports -31.9%   nesting -29.2%
    flat +2.2%   small @media +3.3%   (both inside the +/-5% this box resolves)

Output and source map stay byte for byte main's, streamed or not.
`streamBlocks` was an opt-in because streaming used to be walk-only, and a
caller that printed had to decline it. Printing streams too now, so there is
nothing left for the option to trade off: a block under the threshold is
still collected and walked in one batch exactly as it always was, and one
past it streams. So the walk always streams and the option is gone.

It stays a flag internally, because the standalone `parseA*` entry points
hand back a materialized tree and must not stream; `_setupParse` leaves it
off for them and only `grammar` turns it on.

What a streamed rule reports about its own block is now the walk's ordinary
behaviour rather than something a caller asked for, so `A.declarations` and
`A.childRules` say so where they are defined: they read as an empty block on
a streamed rule, and `null` still means no block at all.

The tests can no longer print a stylesheet both ways to compare, so they
assert the output itself, over fixtures already in minimal form. Every case
pins whether it is exercising the streamed or the collected path, so none of
them can quietly stop testing streaming if the threshold moves.
Generated positions are worked out when a map is asked for rather than as
the output is built, by walking the output alongside the mappings. But the
walk asked `indexOf` for the next newline once per mapping, and `indexOf`
has no end bound: with no newline ahead it scans to the end of the piece and
returns -1, every time. Minified output has no newlines at all, so every one
of a piece's mappings scanned the whole piece -- quadratic in the mappings a
piece holds, which is one per top-level rule.

Carry the next newline's offset across mappings instead, and look for
another only after one is passed. A piece with no newlines is scanned once.

Minifying 10k rules, each arm in its own process, median of 9: flat +2.7%
-> -2.9%, many small `@media` +3.6% -> -5.8%.
@alexander-akait
alexander-akait force-pushed the perf/css-stream-nested-blocks branch from cf02cea to 1b8718b Compare August 9, 2026 20:02
@coderabbitai

coderabbitai Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@alexander-akait, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1025daf4-aa47-46ce-a785-7d9d57a10dd0

📥 Commits

Reviewing files that changed from the base of the PR and between b09558a and aef7cf8.

⛔ Files ignored due to path filters (1)
  • types.d.ts is excluded by !types.d.ts
📒 Files selected for processing (3)
  • lib/css/syntax.js
  • lib/util/SourceProcessor.js
  • test/CssSyntax.unittest.js

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key: "tools"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

Changes

The pull request adds threshold-based streaming for large nested CSS blocks, chunked printer output with source-map support, and tests for streamed traversal and printing. It also updates Bun test worker memory settings and shutdown documentation.

CSS block streaming

Layer / File(s) Summary
Chunked printer output
lib/util/SourceProcessor.js
PrintContext stores output in chunks, supports pending and retractable pieces, and resolves source-map anchors from chunk positions.
Nested block streaming
lib/css/syntax.js
Large nested blocks publish frames, walk completed children incrementally, preserve indexes, and reset streaming state during cleanup.
Rule serialization and accessors
lib/css/syntax.js
Rule prelude serialization is separated from body processing. Streamed rule accessors document empty materialized bodies.
Streaming behavior validation
test/CssSyntax.unittest.js
Tests cover thresholds, traversal, nested blocks, output, declaration handling, comments, and processor cleanup.

Bun worker lifecycle

Layer / File(s) Summary
Bun restart and shutdown handling
.github/workflows/test.yml, package.json
Workflow comments distinguish restart exits from normal shutdown. The Bun Jest command raises the worker idle memory limit to 2048 MB.

Possibly related PRs

  • webpack/webpack#21646: Both changes modify Bun worker memory configuration and worker termination handling.

Suggested labels: area: build-perf

Suggested reviewers: bjohansebas

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title uses valid Conventional Commit syntax and accurately describes the main performance change, but the branch prefix is not provided for verification. Provide the branch name or prefix, or confirm that the branch prefix is perf.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added area: css CSS support (lib/css) area: types types.d.ts, JSDoc annotations, hand-maintained declarations labels Aug 9, 2026
@alexander-akait
alexander-akait marked this pull request as ready for review August 9, 2026 20:04
@coderabbitai coderabbitai Bot added the area: build-perf Build time and memory of webpack itself label Aug 9, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/patches/jest-worker+30.4.1.patch (1)

32-35: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Do not suppress Bun preload failures.

test/bun-preload.js provides required Jest compatibility in Bun worker threads. If it fails to load, this empty catch starts the worker without that setup. Let the worker fail at bootstrap so the CI log shows the actual cause.

Proposed fix
 if (process.versions.bun) {
-	try {
-		require(require("path").resolve(process.cwd(), "test/bun-preload.js"));
-	} catch (e) {}
+	require(require("path").resolve(process.cwd(), "test/bun-preload.js"));
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/patches/jest-worker`+30.4.1.patch around lines 32 - 35, Remove the empty
catch around the Bun preload require so failures from loading
test/bun-preload.js propagate during worker bootstrap. Keep the
process.versions.bun guard and existing preload resolution unchanged.
🧹 Nitpick comments (3)
test/CssSyntax.unittest.js (2)

1024-1046: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the _STREAM_MAX_DEPTH fallback.

_streamConsumeBlock in lib/css/syntax.js (Lines 4402-4413) falls back to the materializing path when d >= _STREAM_MAX_DEPTH. No case in this describe block nests 64 rules deep, so that branch is untested. The fallback sets _setBody, _setBlock, and _setEnd on its own and never publishes a frame, so a regression there would not surface in the current suite.

A case that nests 70 qualified rules and places BIG at the innermost level exercises both the fallback and the streamed levels above it.

As per coding guidelines for test/**/*.{js,json}: "Cover every changed line and branch with tests, preferably through integration cases; do not lower patch coverage below the project target."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/CssSyntax.unittest.js` around lines 1024 - 1046, Add a test case in the
existing state-cleanup describe block that builds roughly 70 nested qualified
rules with BIG at the innermost level, then processes and validates the result
through the normal walk/minify assertions. Ensure the nesting reaches the
_STREAM_MAX_DEPTH fallback in _streamConsumeBlock while preserving coverage for
state cleanup after processing.

Source: Coding guidelines


761-776: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin the streamed path for the BIG cases that do not assert it.

The comment at Lines 762-766 states that every case pins which path it exercises. Four cases do not: Lines 839-846, 848-870, 898-905, and 955-964 use BIG but never assert that the block streamed. _STREAM_MIN_NODES is an internal constant. If it rises, these cases fall back to the collected path and still pass, so they stop testing streaming without failing.

Cases at Lines 971, 988, and 1014 already pin the path with childCount. Apply the same guard to the BIG cases, or assert once that BIG streams.

💚 Proposed guard
 	const BIG = repeat(1800, rule);
 	const SMALL = repeat(4, rule);
+
+	it("BIG crosses the streaming threshold and SMALL does not", () => {
+		// Guards every case below that uses these two fixtures.
+		expect(childCount(`@media screen{${BIG}}`)).toBe(0);
+		expect(childCount(`@media screen{${SMALL}}`)).toBe(4);
+	});

As per coding guidelines for test/**/*.{js,json}: "Cover every changed line and branch with tests, preferably through integration cases; do not lower patch coverage below the project target."

Also applies to: 839-846

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/CssSyntax.unittest.js` around lines 761 - 776, Update the BIG-based test
cases around describe("CssSyntax — block streaming") that currently lack a
streaming assertion, including the cases near Lines 839, 848, 898, and 955. Add
the same childCount-based guard used by the existing pinned cases, or add one
shared assertion that verifies BIG takes the streamed path, so these tests fail
if the threshold changes.

Source: Coding guidelines

lib/util/SourceProcessor.js (1)

319-339: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Skip the push when _tail is empty.

emitRetractable calls _cutTail on every call. In a streamed block each declaration goes through emitRetractable, so the tail is empty for every call after the first one. _cutTail then pushes "" into _chunks, which doubles the array growth for declaration-heavy streamed blocks. charCodeAt(0) on "" also returns NaN, so the flattening read does nothing there.

A guard keeps _chunks at one entry per retractable piece and preserves the chunk-index contract used by anchor and mark, because an empty tail contributes no characters.

♻️ Proposed guard
 	_cutTail() {
+		if (this._tailLength === 0) return;
 		// Reading a character is what forces it flat; the value is not wanted, and
 		// it is kept only so the read cannot be optimized away.
 		this._flattened = this._tail.charCodeAt(0);
 		this._chunks.push(this._tail);
 		this._tail = "";
 		this._tailLength = 0;
 	}

Note: _streamClose calls dropTrailing(_frameFirstChunk[d], …) in lib/css/syntax.js, which walks back over empty pieces, so removing the empty pieces does not change its result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/util/SourceProcessor.js` around lines 319 - 339, Update _cutTail so it
only flattens, appends _tail to _chunks, and resets tail state when _tail is
non-empty; leave the state unchanged when empty. Preserve emitRetractable’s
one-entry-per-retractable-piece indexing contract used by anchor and mark.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/css/syntax.js`:
- Around line 4143-4160: Update the streamed-rule entry flow around _streamOpen
and _streamEnterRule so _currentParent is assigned from the active parent
unconditionally before _rulePrelude runs, including when no enter visitor
exists. Preserve correct path.parent resolution for from-selector rewriting, and
add coverage for streamed `@keyframes` rules.

---

Outside diff comments:
In `@test/patches/jest-worker`+30.4.1.patch:
- Around line 32-35: Remove the empty catch around the Bun preload require so
failures from loading test/bun-preload.js propagate during worker bootstrap.
Keep the process.versions.bun guard and existing preload resolution unchanged.

---

Nitpick comments:
In `@lib/util/SourceProcessor.js`:
- Around line 319-339: Update _cutTail so it only flattens, appends _tail to
_chunks, and resets tail state when _tail is non-empty; leave the state
unchanged when empty. Preserve emitRetractable’s one-entry-per-retractable-piece
indexing contract used by anchor and mark.

In `@test/CssSyntax.unittest.js`:
- Around line 1024-1046: Add a test case in the existing state-cleanup describe
block that builds roughly 70 nested qualified rules with BIG at the innermost
level, then processes and validates the result through the normal walk/minify
assertions. Ensure the nesting reaches the _STREAM_MAX_DEPTH fallback in
_streamConsumeBlock while preserving coverage for state cleanup after
processing.
- Around line 761-776: Update the BIG-based test cases around
describe("CssSyntax — block streaming") that currently lack a streaming
assertion, including the cases near Lines 839, 848, 898, and 955. Add the same
childCount-based guard used by the existing pinned cases, or add one shared
assertion that verifies BIG takes the streamed path, so these tests fail if the
threshold changes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d50ec46d-6a4c-4218-b278-6d721eed4761

📥 Commits

Reviewing files that changed from the base of the PR and between 6412396 and 769753e.

⛔ Files ignored due to path filters (1)
  • types.d.ts is excluded by !types.d.ts
📒 Files selected for processing (6)
  • .github/workflows/test.yml
  • lib/css/syntax.js
  • lib/util/SourceProcessor.js
  • package.json
  • test/CssSyntax.unittest.js
  • test/patches/jest-worker+30.4.1.patch
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • webpack/schema-utils (auto-detected)
  • webpack/tapable (auto-detected)

Comment thread lib/css/syntax.js
`runtimes (bun)` segfaults at a different wild address each run, on this
branch and on main alike. Reproduced locally with Bun 1.3.14, the CI patches
and the CI command, and it is the idle-memory restart: with the limit below
what the suite uses, workers restart constantly and Bun crashes tearing one
down.

  limit  vs the suite's heaps  result
   16MB  below                 segfault, spawned(61) terminated(57)
   64MB  below                 segfault, spawned(20) terminated(16)
  512MB  above                 103 suites, 28993 tests, green
 2048MB  above                 103 suites, 28993 tests, green

The last green CI run had no limit at all and finished all 159 suites with
worker heaps reaching 1337 MB. Restoring the limit set it to the 512 MB the
other runtimes use — under half of that — so it fires on workers doing
ordinary work, and the run dies about a third of the way in. Raise it to
2048 MB: above what the suite needs, still a bound on a worker that runs
away. Node and Deno keep 512 MB, where a spurious restart only costs time.

Ending a worker rather than terminating it, as the patch already does, buys
Bun a restart it would not otherwise survive — a small run took five and
passed — but not a run full of them. Both teardown paths crash in bulk, so
the limit is the lever, and the patch is left as it is.
@alexander-akait
alexander-akait force-pushed the perf/css-stream-nested-blocks branch from 769753e to 699b011 Compare August 9, 2026 20:44
A prelude can be read in terms of what encloses it: `from` is the `0%` a
keyframe selector means, but only inside `@keyframes`. `_streamOpen` bound
the node and left the parent as whatever the last walked node had, because
`_streamEnterRule` binds it only when the rule has an `enter` visitor to run
— which printing on its own does not. A `@keyframes` selector whose own
block crossed the threshold therefore printed as `from` where the collected
printer prints `0%`, and the two disagreed on the same input.

Also stop cutting an empty piece off the output. Every retractable piece
cuts the accumulated tail off in front of it, so a run of declarations in a
streamed block cut an empty one for each after the first: 3000 consecutive
declarations built 6000 pieces where 3001 carry the whole output.

Both found by CodeRabbit on the pull request, and both now covered: the
keyframe case, and the fallback past the depth the frame table holds, which
nothing reached before.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/util/SourceProcessor.js (1)

233-238: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Cut the tail before capturing writer.mark. mark stores only _chunks.length. If _tail contains preceding output, emitRetractable() moves it into the chunk at mark. After the marked piece is retracted, dropTrailing(mark, ...) can remove a trailing character from that preceding output. Add a regression test for preceding tail output and a retracted final piece.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/util/SourceProcessor.js` around lines 233 - 238, The mark getter must
account for pending _tail output before capturing the next emitted piece’s
position. Update the mark/emitRetractable flow in SourceProcessor so preceding
tail content is cut into chunks before reading writer.mark, ensuring
dropTrailing(mark, ...) cannot retract characters from earlier output; add a
regression test covering preceding tail output followed by a retracted final
piece.
🧹 Nitpick comments (1)
lib/util/SourceProcessor.js (1)

320-324: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten the new _cutTail comment.

The comment spans four lines. Keep only the reason for the early return and fit it in two short lines.

Proposed comment
-		// Nothing has accumulated since the last cut — which is every retractable
-		// piece after the first in a run of them. A piece holding no characters
-		// would only pad the output, and an anchor's offset into an empty tail is
-		// the start of whatever is cut next either way.
+		// Skip empty tails to avoid padding chunks.
+		// Anchor offsets remain unchanged.

As per coding guidelines, comments under lib/ must be at most two short lines and add non-obvious information. As per path instructions, comments under lib/** must be at most two short lines and add non-obvious information.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/util/SourceProcessor.js` around lines 320 - 324, Shorten the comment
immediately above the _tailLength === 0 early return in _cutTail to no more than
two short lines, retaining only the non-obvious reason for returning when no
characters have accumulated.

Sources: Coding guidelines, Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@lib/util/SourceProcessor.js`:
- Around line 233-238: The mark getter must account for pending _tail output
before capturing the next emitted piece’s position. Update the
mark/emitRetractable flow in SourceProcessor so preceding tail content is cut
into chunks before reading writer.mark, ensuring dropTrailing(mark, ...) cannot
retract characters from earlier output; add a regression test covering preceding
tail output followed by a retracted final piece.

---

Nitpick comments:
In `@lib/util/SourceProcessor.js`:
- Around line 320-324: Shorten the comment immediately above the _tailLength ===
0 early return in _cutTail to no more than two short lines, retaining only the
non-obvious reason for returning when no characters have accumulated.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d86fbc8-a32d-4b5d-84b6-f0b06cf55daa

📥 Commits

Reviewing files that changed from the base of the PR and between 699b011 and b09558a.

📒 Files selected for processing (3)
  • lib/css/syntax.js
  • lib/util/SourceProcessor.js
  • test/CssSyntax.unittest.js
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • webpack/schema-utils (auto-detected)
  • webpack/tapable (auto-detected)
🚧 Files skipped from review as they are similar to previous changes (2)
  • test/CssSyntax.unittest.js
  • lib/css/syntax.js

The `;` a `}` makes redundant is dropped by walking the output back from the
end, bounded by where the block started. That bound was taken while the
block's opener was still held back, so the piece it names can hold output
from before the block — the walk stops short of it today only because the
opener's piece ends in `{` and is never taken back, which is an invariant
nothing states. Take the bound after cutting the tail, so it names the
block's own first piece and the walk cannot reach past it either way.

The bound now covers a case it did not: a run of retractable pieces already
cuts after each, so a mark taken between them was mid-tail.

Raised by CodeRabbit as a correctness risk. It is not reachable today —
reverting either the cut or the bound entirely leaves every test passing —
so this is the invariant made explicit rather than a fix. The new case pins
what is observable: a declaration before a streamed block keeps its `;` when
the block drops its own.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Types Coverage

Coverage after merging perf/css-stream-nested-blocks into main will be
99.29%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
bin
   webpack.js98.82%100%100%98.82%103
examples
   build-common.js100%100%100%100%
   buildAll.js100%100%100%100%
   examples.js100%100%100%100%
   template-common.js98.21%100%100%98.21%72
examples/custom-javascript-parser
   test.filter.js100%100%100%100%
examples/custom-javascript-parser/internals
   acorn-parse.js100%100%100%100%
   meriyah-parse.js100%100%100%100%
   oxc-parse.js100%100%100%100%
examples/markdown
   webpack.config.mjs100%100%100%100%
examples/module-federation
   test.filter.js100%100%100%100%
examples/reexport-components
   test.filter.js100%100%100%100%
examples/typescript
   test.filter.js100%100%100%100%
examples/typescript-non-erasable
   test.filter.js50%100%100%50%5
examples/virtual-modules
   test.filter.js100%100%100%100%
examples/wasm-bindgen-esm
   test.filter.js100%100%100%100%
examples/wasm-complex
   test.filter.js100%100%100%100%
examples/wasm-emscripten
   test.filter.js100%100%100%100%
examples/wasm-simple
   test.filter.js100%100%100%100%
examples/wasm-simple-source-phase
   test.filter.js100%100%100%100%
lib
   APIPlugin.js100%100%100%100%
   AsyncDependenciesBlock.js100%100%100%100%
   AutomaticPrefetchPlugin.js100%100%100%100%
   BannerPlugin.js100%100%100%100%
   Cache.js98.21%100%100%98.21%101
   CacheFacade.js100%100%100%100%
   Chunk.js99.72%100%100%99.72%39
   ChunkGraph.js100%100%100%100%
   ChunkGroup.js100%100%100%100%
   ChunkTemplate.js100%100%100%100%
   CircularModulesPlugin.js98.81%100%100%98.81%136
   CleanPlugin.js99.13%100%100%99.13%214, 234
   CodeGenerationResults.js100%100%100%100%
   CompatibilityPlugin.js100%100%100%100%
   Compilation.js98.42%100%100%98.42%1648, 1967, 1974, 1982, 2004, 2007, 2946, 3425–3426, 3458, 4162, 4192, 4245–4246, 4250, 4255, 4271–4272, 4286–4287, 4292–4293, 4773, 4799, 527, 532, 5607, 5639, 5656, 5672, 5688, 5703, 5728–5729, 5731, 6061, 6066, 6072, 6075, 6082, 6094, 6096, 6100, 6116, 6131, 6163, 6217, 6241, 6356, 778–779
   Compiler.js99.56%100%100%99.56%1159–1160, 1168
   ConcatenationScope.js99.12%100%100%99.12%300
   ConditionalInitFragment.js100%100%100%100%
   ConstPlugin.js100%100%100%100%
   ContextExclusionPlugin.js100%100%100%100%
   ContextModule.js99.88%100%100%99.88%1461
   ContextModuleFactory.js97.20%100%100%97.20%266, 435, 456, 461, 501, 512, 514, 518, 527–528
   ContextReplacementPlugin.js100%100%100%100%
   DefinePlugin.js99.08%100%100%99.08%1080, 176–177, 193, 212, 286
   DependenciesBlock.js100%100%100%100%
   Dependency.js98.51%100%100%98.51%480, 527
   DependencyTemplate.js100%100%100%100%
   DependencyTemplates.js100%100%100%100%
   DotenvPlugin.js98.41%100%100%98.41%378, 391–392
   DynamicEntryPlugin.js100%100%100%100%
   EntryOptionPlugin.js100%100%100%100%
   EntryPlugin.js100%100%100%100%
   Entrypoint.js100%100%100%100%
   EnvironmentPlugin.js97.14%100%100%97.14%49
   ErrorHelpers.js100%100%100%100%
   EvalDevToolModulePlugin.js100%100%100%100%
   EvalSourceMapDevToolPlugin.js100%100%100%100%
   ExportsInfo.js100%100%100%100%
   ExportsInfoApiPlugin.js100%100%100%100%
   ExternalModule.js98.67%100%100%98.67%1216, 1219, 515–519, 521, 667
   ExternalModuleFactoryPlugin.js100%100%100%100%
   ExternalsPlugin.js100%100%100%100%
   FileSystemInfo.js99.16%100%100%99.16%1267, 1269–1274, 1281, 1284, 182, 2502–2503, 2506, 2517, 2528, 2539, 280, 3976, 3991, 4015
   FlagAllModulesAsUsedPlugin.js100%100%100%100%
   FlagDependencyExportsPlugin.js98.36%100%100%98.36%504, 513, 516, 520, 532
   FlagDependencyUsagePlugin.js100%100%100%100%
   FlagEntryExportAsUsedPlugin.js100%100%100%100%
   Generator.js100%100%100%100%
   HotModuleReplacementPlugin.js100%100%100%100%
   HotUpdateChunk.js100%100%100%100%
   IgnorePlugin.js100%100%100%100%
   IgnoreWarningsPlugin.js100%100%100%100%
   InitFragment.js100%100%100%100%
   JavascriptMetaInfoPlugin.js100%100%100%100%
   LazyBarrel.js100%100%100%100%
   LibraryTemplatePlugin.js100%100%100%100%
   LoaderOptionsPlugin.js100%100%100%100%
   LoaderTargetPlugin.js100%100%100%100%
   MainTemplate.js100%100%100%100%
   ManifestPlugin.js100%100%100%100%
   Module.js98.51%100%100%98.51%1303, 1308, 1368, 1382, 1444, 1453
   ModuleFactory.js100%100%100%100%
   ModuleFilenameHelpers.js98.90%100%100%98.90%111, 113
   ModuleGraph.js99.78%100%100%99.78%1170
   ModuleGraphConnection.js100%100%100%100%
   ModuleInfoHeaderPlugin.js100%100%100%100%
   ModuleNotFoundError.js100%100%100%100%
   ModuleProfile.js100%100%100%100%
   ModuleSourceTypeConstants.js100%100%100%100%
   ModuleTemplate.js100%100%100%100%
   ModuleTypeConstants.js100%100%100%100%
   MultiCompiler.js99.72%100%100%99.72%716
   MultiStats.js100%100%100%100%
   MultiWatching.js100%100%100%100%
   NoEmitOnErrorsPlugin.js100%100%100%100%
   NodeStuffPlugin.js100%100%100%100%
   NormalModule.js97.98%100%100%97.98%1020, 1023, 1040, 1057, 1305, 1339, 1355, 1803, 2100, 2105–2115, 34
   NormalModuleFactory.js98.72%100%100%98.72%1123, 1391, 1402, 1412, 1463–1465, 1472, 526, 538
   NormalModuleReplacementPlugin.js100%100%100%100%
   NullFactory.js100%100%100%100%
   OptimizationStages.js100%100%100%100%
   OptionsApply.js100%100%100%100%
   Parser.js100%100%100%100%
   PlatformPlugin.js100%100%100%100%
   PrefetchPlugin.js100%100%100%100%
   ProgressPlugin.js99.80%100%100%99.80%691
   ProvidePlugin.js100%100%100%100%
   RawModule.js100%100%100%100%
   RecordIdsPlugin.js100%100%100%100%
   RequestShortener.js100%100%100%100%
   ResolverFactory.js100%100%100%100%
   RuntimeGlobals.js100%100%100%100%
   RuntimeModule.js100%100%100%100%
   RuntimePlugin.js100%100%100%100%
   RuntimeTemplate.js100%100%100%100%
   SelfModuleFactory.js100%100%100%100%
   SingleEntryPlugin.js100%100%100%100%
   SourceMapDevToolModuleOptionsPlugin.js100%100%100%100%
   SourceMapDevToolPlugin.js98.63%100%100%98.63%220, 224, 226, 420, 431, 890
   Stats.js100%100%100%100%
   Template.js100%100%100%100%
   TemplatedPathPlugin.js99.47%100%100%99.47%352–353
   UseStrictPlugin.js100%100%100%100%
   WarnCaseSensitiveModulesPlugin.js100%100%100%100%
   WarnDeprecatedOptionPlugin.js100%100%100%100%
   WarnNoModeSetPlugin.js100%100%100%100%
   WatchIgnorePlugin.js100%100%100%100%
   Watching.js100%100%100%100%
   WebpackError.js100%100%100%100%
   WebpackIsIncludedPlugin.js100%100%100%100%
   WebpackOptionsApply.js100%100%100%100%
   WebpackOptionsDefaulter.js100%100%100%100%
   buildChunkGraph.js99.87%100%100%99.87%371
   cli.js98.63%100%100%98.63%10, 119, 549, 581, 631, 905
   index.js99.73%100%100%99.73%184
   validateSchema.js94.67%100%100%94.67%100, 87, 89, 98
   webpack.js97.10%100%100%97.10%10, 263, 285, 287
lib/asset
   AssetBytesGenerator.js100%100%100%100%
   AssetBytesParser.js100%100%100%100%
   AssetGenerator.js100%100%100%100%
   AssetModule.js100%100%100%100%
   AssetModulesPlugin.js97.95%100%100%97.95%295, 319, 322, 42, 452, 47
   AssetParser.js100%100%100%100%
   AssetSourceGenerator.js100%100%100%100%
   AssetSourceParser.js100%100%100%100%
   RawDataUrlModule.js100%100%100%100%
   WebManifestGenerator.js100%100%100%100%
   WebManifestParser.js100%100%100%100%
lib/async-modules
   AsyncModuleHelpers.js100%100%100%100%
   AwaitDependenciesInitFragment.js100%100%100%100%
   InferAsyncModulesPlugin.js100%100%100%100%
   isGeneratorLowered.js100%100%100%100%
lib/bun
   BunTargetPlugin.js100%100%100%100%
lib/cache
   AddBuildDependenciesPlugin.js100%100%100%100%
   AddManagedPathsPlugin.js100%100%100%100%
   IdleFileCachePlugin.js97.92%100%100%97.92%75, 87, 95
   MemoryCachePlugin.js92%100%100%92%34, 43
   MemoryWithGcCachePlugin.js93.42%100%100%93.42%108, 122–123, 132, 90
   PackFileCacheStrategy.js96.52%100%100%96.52%1310, 1410, 1414, 1476, 1712, 1796, 1819, 1851, 675, 694, 704–706, 708, 724–725, 730, 733, 735, 740, 745, 770, 776, 810, 816, 822, 827, 838, 847, 852–853, 855, 872, 878–879, 881
   ResolverCachePlugin.js100%100%100%100%
   

@alexander-akait
alexander-akait merged commit ce12f4e into main Aug 9, 2026
64 checks passed
@alexander-akait
alexander-akait deleted the perf/css-stream-nested-blocks branch August 9, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: build-perf Build time and memory of webpack itself area: css CSS support (lib/css) area: types types.d.ts, JSDoc annotations, hand-maintained declarations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant