Skip to content

perf(html): stream the SourceProcessor walk and recycle completed subtrees - #21595

Merged
alexander-akait merged 24 commits into
mainfrom
perf/html-stream-source-processor-walk
Aug 10, 2026
Merged

alexander-akait merged 24 commits into
mainfrom
perf/html-stream-source-processor-walk

Conversation

@alexander-akait

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

Copy link
Copy Markdown
Member

Summary

html.syntax's SourceProcessor built the whole document before walking it, so peak node storage scaled with the document (a 2 MiB page held ~10 MiB of columns across 112k nodes at a tree depth of 8). The walk now follows the parser's open element stack and recycles each completed subtree, which is what the CSS grammar already does per top-level rule.

What kind of change does this PR introduce?

perf

Did you add tests for your changes?

Yes — test/HtmlSyntax.unittest.js covers final end offsets in the streamed walk; the existing configCases/html suites cover the parser and generator paths.

Does this PR introduce a breaking change?

No. One nuance on the experimental SourceProcessor API: an element that stays open across a flush reports its start-tag end at enter rather than its full span. This affects 14 of 25,612 differential parses, all misnested <li>; parseHtml is unchanged.

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

n/a — SourceProcessor is @experimental and undocumented.

Use of AI

Yes. Claude Code was used to implement the change, write the benchmarks and the differential/fuzz harnesses, and run the measurements; every result quoted here was measured rather than estimated, and the author directed and reviewed the work.


Generated by Claude Code


Note

Medium Risk
Large refactor of HTML parse/walk internals with experimental SourceProcessor behavior changes (e.g. provisional end on long-open elements); core parseHtml and printing paths are largely unchanged but adoption-agency and table edge cases rely on new halt/sync logic.

Overview
Walk-only SourceProcessor passes now stream the visitor walk during parseHtml instead of waiting until EOF, recycling node ids for completed subtrees under the parser’s open stack—similar to the CSS grammar’s per-rule recycling. Streaming kicks in past ~49k nodes; printing still builds the full tree and walks once at the end.

Parser plumbing: popOpen / removeOpenAt centralize open-stack changes so each token can sync the walk; mid-stack open.splice halts streaming when reconciliation would be unsafe. _nodeHighWater tracks the highest node id when ids are recycled so AST column release clears every slot that was written.

HtmlGenerator.collectHtml collects script / link / style / meta on element exit so spans use final end offsets under the streamed walk.

Tests cover large-document streaming, skipChildren, text merging across flush boundaries, </form> mid-stack removal, tables, and head element offsets.

Reviewed by Cursor Bugbot for commit be31281. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Performance
    • Improved processing and printing of large HTML documents through incremental streaming.
    • Reduced memory usage by recycling internal parsing data where possible.
  • Bug Fixes
    • Improved handling of document regions, nested elements, templates, tables, self-closing elements, and omitted tags.
    • Preserved whitespace, formatting, and minification behavior during streamed output.
    • Improved handling of incomplete markup, comments, doctypes, CDATA, scripts, and character references.
  • Reliability
    • Added broader coverage for complex HTML structures and streaming edge cases.
    • Improved parser recovery and error handling for malformed or incomplete HTML.

…trees

Visit and release each completed subtree during tree construction instead of
materializing the whole document first, so peak node storage tracks the open
subtree rather than the document. Flushing is gated on an empty active
formatting elements list, which also covers table markers, so neither the
adoption agency nor foster parenting can reach an already-flushed node;
unsafe stretches buffer and the next catch-up sees the final tree.

Skip the input-length column pre-sizing when streaming — it allocated orders
of magnitude more than a streamed parse ever uses.

Only walk-only passes stream; printing holds every node's text until its
parent prints, so `minimize` still builds the tree and walks it. `parseHtml`
itself is unchanged.

On the 2.33 MiB benchmark document with `skip.text`: peak live AST
10.19 -> 1.26 MiB, parse+walk 125.1 -> 104.6 ms.

INCOMPLETE — do not merge. A pre-order `enter` cannot observe post-order
facts, so an element still on the open stack reports a provisional `end`,
`firstChild` and `nextSibling`. HtmlGenerator and HtmlParser read `end` on
Element enter for the head/body injection anchors and must move to `exit`,
and HtmlSyntax.unittest.js currently has 1 failing case (`firstChild`).
Bail out of the sync before the safety check on the common token shape (open
stack untouched, nothing completed under the current element), and replace the
O(depth) reconcile scan with a suffix scan, falling back to the full scan only
when a mid-stack open.splice marks the stack dirty.

Corrects the measurement in the previous commit. That 125.1 -> 104.6 ms
compared two separate process runs and does not survive a controlled A/B; in
child-process runs (median of 7) the streamed walk is still slower than the
buffered one, by ~8-25% depending on document shape, after these two fixes cut
it from ~25-35%. Memory is a large win: peak column bytes drop 54-85%.

Still INCOMPLETE - the provisional-end breakage is unfixed.
An element is entered while it is still open, so its end offset and child
links are not final there. Move HtmlGenerator's head/body injection anchors
and its script/link/style/meta collection to exit, where end is final;
script/link/style/meta never nest in each other, so source order is kept.

Cover the streamed contract in the path-accessor test: a completed child
subtree is visited and unlinked while its parent is still open, so child
links read empty from either half of an element's visit - structure is read
from parseHtml's materialized tree instead.

HtmlParser's headLastEnd still reads end on enter. Moving it to exit
regressed 25 CSS-link injection cases, so it needs its own investigation.
A flush only earns its walk setup once enough has piled up under the current
element, so the per-token cost drops from a flush attempt to a subtraction.

Batching alone corrupted the tree: deferring flushes let the allocator rewind
below an element that was still open, so the next mkEl aliased a live node and
produced self-referential links (parent === firstChild === itself), which spun
the walk forever. Route every rewind through _streamRewind, which never
reclaims an id at or below the deepest open element, and only relocate a
held-back trailing text node when it is the sole survivor above its parent so
the id space keeps no gaps.

Found with a temporary link validator (cycle / bad-parent / open-above-count)
run over the html5lib tokenizer corpus plus 6000 seeded fuzz documents:
25612 parses, 0 violations. The differential against the buffered walk is
unchanged at 590 mismatches, now all the inherent end-at-enter class.
Key the per-token gate on the deepest *entered* element rather than the
deepest open one. The open stack changes on every start and end tag, so the
old gate never engaged on shallow markup; keyed this way, an element that
opens and closes inside the batch window is never entered at all and is later
walked as one completed subtree. That is both cheaper and gives its visitors a
final end offset - the differential against the buffered walk drops from 590
mismatches to 14.

Flush a run of children with a single walk instead of a call per child, pick
the run's stop node in O(1) from lastChild, and inline the gate into dispatch
so an idle token costs three loads and a compare.

Batch 4096 balances the two: on the nested benchmark document 6-15% faster
than the buffered walk with ~85% less peak column memory; deep nesting is
neutral. Pathologically flat markup (60k siblings under body) is still 5-13%
slower - what is left there is the per-token gate itself, which stays even
when the batch is raised until the memory win is gone.
A streamed parse recycles each flushed subtree, so it was left to grow from
the default capacity by doubling. A document that keeps everything open (deep
nesting) still reaches the document's size, and doubling into it overshoots
where one exact allocation would not - 5000 nested divs peaked at 0.74 MiB of
columns against the buffered walk's 0.43 MiB.

Apply the same input-length estimate, capped at what one batch can hold, so
deep nesting matches the buffered peak exactly and wide documents keep the
small streamed footprint.
…elocate

_walkSubtree was _walkRun restricted to a single subtree - the printing path
now calls _walkRun directly with the node's own parent and next sibling as the
run bounds, so one walker covers both.

The relocate existed to close the id-space gap a held-back trailing text node
left behind. Since every rewind is clamped by _streamRewind that gap can no
longer be reused by a live node, and dropping it changes nothing measurable:
same 14 differential mismatches, same 2.35 MiB peak, same timings.
Route every open-stack pop through popOpen, which marks the walk dirty, and
have dispatch do one boolean test per token instead of re-deriving the whole
gate. Only a stack change can complete a subtree, so the four-term gate was
being recomputed on every token to answer a question that popOpen already
knows - the CSS grammar gets the same effect from its onRule sink.

The mark cannot do the work itself: an element's end offset is assigned after
it is popped, so syncing inside popOpen visits it with a provisional end (it
regressed the differential to a new mismatch class). The flag defers to the
end of the token, where the end is final.

This closes the flat-markup regression - 60k siblings under body go from
~13-20% slower than the buffered walk to ~6% faster, with peak columns still
roughly halved.
HtmlParser folds a head element's end into its injection anchor at enter and
consumes that anchor in the same visit, so a provisional end there would
misplace injected tags. The streamed walk only starts once the parse reaches
in-body, which is what keeps every head element a completed subtree with a
final end - assert it directly so a change to that guard cannot silently
break head injection.

Restore the path-accessor test to its original enter-only form: entering
lazily and driving the walk from the pops means an element is walked as a
completed subtree, so its child links are live again at enter.
Collapse _streaming / _streamRequested into one flag whose lifetime grammar
owns through try/finally, so a thrown parse can no longer leak streaming into
a later direct parseHtml call and the consume-once dance disappears.

_openSpliced already implies a stack change, so the five sites that set both
now set one and dispatch ORs them.

Split _streamSync into named steps (prefix, close, open, flush) behind a
comment that states the model up front: the walk follows the open element
stack, an element is entered only when something beneath it must be flushed,
and nothing is visited unless _canFlush() holds. Rename _streamSafe to
_canFlush and _streamDirty to _openStackChanged so the vocabulary matches.

No behaviour change: differential unchanged at 14, 452 unit tests pass,
timings within noise.
@changeset-bot

changeset-bot Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 15827d4

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 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Install it locally:

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

@codecov

codecov Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.93333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.50%. Comparing base (57f8dcf) to head (15827d4).

Files with missing lines Patch % Lines
lib/html/syntax.js 98.89% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #21595      +/-   ##
==========================================
- Coverage   94.51%   94.50%   -0.01%     
==========================================
  Files         630      630              
  Lines       81583    81830     +247     
  Branches    24118    24204      +86     
==========================================
+ Hits        77108    77336     +228     
- Misses       4475     4494      +19     
Flag Coverage Δ
css-parsing 26.37% <0.00%> (-0.01%) ⬇️
html5lib 30.83% <55.73%> (+0.05%) ⬆️
integration 88.35% <75.73%> (-0.06%) ⬇️
test262 50.97% <ø> (-0.01%) ⬇️
unit 54.46% <98.93%> (+0.13%) ⬆️

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.

Comment thread lib/html/syntax.js Resolved
A descendant of a skipChildren() element is tracked on _entered without being
entered, but the close path called _exitNode on it anyway, so exit fired with
no matching enter once nesting was deep enough to stream mid-parse. The
buffered walk fires exit for the skipped element itself and nothing below it;
match that in both the sync and the EOF paths. Reported by Cursor Bugbot.

Reattach the _streamFlush and _streamRewind JSDoc blocks, which an earlier
edit left stacked above _walkRun - tsc reported five implicit-any parameters
and failed lint.

Add streamed-walk tests sized past the flush batch so the enter, flush,
detach and recycle paths are exercised by the unit suite rather than only by
integration: element visits stay balanced across batches, skipChildren pairs
correctly (this case fails with 3635 unmatched exits without the fix above),
adjacent text stays merged, and forms and tables stream.
@codspeed

codspeed Bot commented Aug 3, 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

⚡ 4 improved benchmarks
❌ 1 regressed benchmark
✅ 257 untouched benchmarks
🆕 48 new benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Memory unit benchmark "html-parser-document-unit", parseHtml (fragment, tr context) 237.8 KB 410.3 KB -42.05%
⚡ Memory unit benchmark "html-parser-document-unit", process (no visitors) 9,727.6 KB 544.6 KB ×18
⚡ Memory unit benchmark "html-parser-document-unit", process (Element+Comment visitors) 9,727.5 KB 544.6 KB ×18
⚡ Memory benchmark "react", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 389.3 KB 233.6 KB +66.64%
⚡ Memory benchmark "concatenate-modules", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 2.8 MB 2.3 MB +22.5%
🆕 Simulation unit benchmark "html-printer-document-unit", beautify (composed subtrees) N/A 327.3 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", beautify (deep nesting) N/A 359.9 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", beautify (document) N/A 927.9 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", beautify (flat siblings) N/A 319.3 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", beautify (fragment, tr context) N/A 526.1 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", beautify (implied start tags) N/A 209.4 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", beautify (nested lists) N/A 468 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", beautify (wide table) N/A 610.2 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", minify (composed subtrees) N/A 380.1 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", minify (deep nesting) N/A 440.6 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", minify (document) N/A 1.1 s N/A
🆕 Simulation unit benchmark "html-printer-document-unit", minify (flat siblings) N/A 408.1 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", minify (fragment, tr context) N/A 633.8 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", minify (implied start tags) N/A 257.5 ms N/A
🆕 Simulation unit benchmark "html-printer-document-unit", minify (nested lists) N/A 583.4 ms N/A
... ... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

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


Comparing perf/html-stream-source-processor-walk (15827d4) with main (57f8dcf)

Open in CodSpeed

Comment thread lib/html/syntax.js Resolved
A mid-stack open.splice takes an element off the stack without ending its
subtree - </form> with a div still open inside it leaves the div live, nested
in the form. Reconciling the entered stack against the open stack there treats
the div as finished: it exits and detaches a live element, then enters it
again, so visitors see a false close/reopen and later inserts hang off a
detached subtree. Reported by Cursor Bugbot.

Those splices are rare, so stop streaming for the rest of the parse instead of
teaching the reconcile to model an insertion point that outlives its stack
entry: what is already visited stays visited, and the remainder is walked at
EOF. The same handling covers the adoption agency's splices, which drops the
differential against the buffered walk from 14 mismatches to 0 - the streamed
walk now matches it exactly across 25612 parses.

The regression test asserts visit counts, not enter/exit pairing: the faulty
sequence (+div -div -form +div) is balanced, so pairing alone cannot see it.
Comment thread lib/html/syntax.js Resolved
Comment thread lib/html/syntax.js Resolved
The halt was set at every open.splice, which was far broader than the problem
it guards. Two common shapes lost subtree recycling for the whole parse:
</form> (any form, even when it is the stack top after implied end tags) and
the afterHead reopen for a late link/meta/script/style. Both reported by
Cursor Bugbot; the form case also showed up in a form-vs-form-free benchmark,
where peak columns went 0.76 -> 4.58 MiB, cancelling the entire win.

Route every stack removal through removeOpenAt, which halts only when the
removed element was actually entered and still has open descendants above it.
Removing the stack top is just a pop, and an element the walk never entered
(anything opened while the active formatting elements list was non-empty, and
everything in the head) costs it nothing.

Forms and late head tags now stream again - both back to 0.76 MiB - and the
re-visit regression test still passes, so the false close/reopen stays fixed.
The differential goes back from 0 to 14 mismatches: the broad halt had masked
them by not streaming at all. They are the inherent enter-time end offset on
misnested li, and event counts match everywhere (0 cases differ), so no
element is visited a different number of times.
Streaming every walk cost more than it saved on ordinary pages: a 72 KiB
page is ~4.5k nodes, so the per-token sync and the flushes ran with nothing
to reclaim. Measured against main on a realistic page shape, that was +6.6%
at 142 KiB and +7.1% at 568 KiB, while the memory win only appeared past
~1 MiB per page.

Start the walk unstreamed and switch it on once the document passes
`_STREAM_MIN_NODES`; the first sync enters the open stack and flushes what
already finished. Below the threshold a page is walked once at EOF, which
is what it wanted anyway.

Also stop capping the attribute estimate for a streamed parse. Attribute
ids are never recycled (`_attrCount` only resets between parses), so the
cap just forced the columns through the doubling cascade. The node cap
stays under `_COLUMN_SHRINK_CAPACITY` so a streamed parse's columns are
still reusable by the next parse.

Peak parser columns over six 2.4 MiB pages: 19.2 MiB -> 5.8 MiB. Ordinary
pages are byte-identical to main in both time and memory.
Comment thread lib/html/syntax.js Resolved
Comment thread lib/html/syntax.js Resolved
Two retention regressions from hoisting the walk to module scope.

`_releaseAstColumns` cleared `_nodeStrings` only up to `_nodeCount`, but
streaming rewinds that counter as it recycles ids, so every string written
above the final count stayed reachable. The streamed capacity is kept under
`_COLUMN_SHRINK_CAPACITY` so the drop-the-array path never ran either.
Measured on a 60k-paragraph document: 49,148 strings / 270,314 characters
pinned after the parse returned, against zero on main. Track the id
high-water mark and clear up to that instead.

`grammar` also left the caller's compiled visitor map and print context in
the module-scope `_visitors` / `_writer` after returning, keeping those
closures alive until the next parse; they used to fall away with the call.
Clear both in the `finally`.
Comment thread lib/html/syntax.js Resolved
The walk-only path called `_enterNode(doc)` for its side effect and threw
the return value away, then finished with `_streamFinish`, which flushes
the root's children with visiting forced on. A `skipChildren()` in the
document's `enter` was therefore ignored and the whole tree was visited —
on every document, not just streamed ones. Against main, a four-element
page went from 2 visits to 12, and a 120k-node one from 2 to 160,008.

Stop the walk when the root's `enter` declines its children, leaving only
the document's own `exit` to fire, which is what the pre-stream walk did.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 657f73d. Configure here.

Comment thread lib/html/syntax.js Resolved
`removeOpenAt` only halted when the removed element was already entered.
An element the walk had not entered yet is just as unsafe: it leaves the
stack while descendants stay open, and the next sync enters those
descendants against an open stack that no longer describes their parent.
A document that crosses the streaming threshold before reaching
`<form><div></form><p>y</p></div>` lost the div and its paragraph
entirely — main visits eight nodes there, the streamed walk visited four.

Key the halt on the mid-stack test alone. Removing the top of the stack,
which is the common `</form>` after implied end tags, still streams: 40k
forms retain 2.80 MiB against main's 9.50 MiB.
@alexander-akait
alexander-akait marked this pull request as draft August 4, 2026 10:08
The printer built each node's text from its children's, so the print store
held every node's string until the root was taken — several times the output
for a document of any size, and the reason this branch's walk streaming is
limited to walk-only passes.

A node whose text is exactly `open + children + close` now emits its opener
as it opens and its closer as it exits, and each child goes straight out as
it finishes; the store only ever holds the subtrees that are not of that
shape (a `<pre>` leading newline to round-trip, a `<template>`'s content
fragment, a `/>` whose end tag depends on what printed inside it). An
omitted `<html>` / `<body>` waits in the writer's pending stack, since it
materializes only when the first thing inside it is whitespace.

The walk is unchanged apart from carrying the open path's depth, which is
what lets a node reach the frame its opener was emitted from.

Output is unchanged: byte-identical over the 225 fixture documents and
50,000 random ones, plus the same source map.

Retained heap at the walk's end, and CPU per print (20k-node documents):

  table  12.51 -> 1.47 MiB   105 -> 77 ms
  flat    5.33 -> 0.03 MiB    33 -> 27 ms
  lists   5.23 -> 0.69 MiB    62 -> 51 ms
  deep    1.54 -> 0.62 MiB   5.5 -> 4.7 ms
@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

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

Summary

The HTML parser now supports streamed AST walking and piecewise printing. It tracks open-stack changes, recycles AST nodes, updates streaming output, and adds tests and benchmarks for large and edge-case inputs.

Changes

HTML streaming pipeline

Layer / File(s) Summary
Parser stack tracking
lib/html/HtmlGenerator.js, lib/html/syntax.js
Body and head region tracking is corrected. Open-stack mutations now use shared helpers across HTML, foreign-content, table, form, and adoption-agency handling.
Streamed walk lifecycle
lib/html/syntax.js
Streaming initializes per parse, tracks recycled node storage, synchronizes stack changes, flushes completed subtrees, and falls back when required.
Piecewise printing output
lib/html/syntax.js, lib/util/SourceProcessor.js
Element tag handling supports streamed eligibility, implied tags, templates, self-closing elements, and minification. PrintContext can update pending output, emit streamed text, and print pieces directly.
Streaming validation and benchmarks
test/HtmlSyntax.unittest.js, test/benchmarkCases/html-printer-document-unit/index.bench.mjs
Tests cover traversal, recycling, offsets, skipped subtrees, table handling, and piecewise printing. Benchmarks cover varied document and fragment structures.

Possibly related PRs

Suggested reviewers: bjohansebas

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses the required Conventional Commit format and accurately describes the streaming and subtree recycling performance changes.
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: html Experimental HTML support (lib/html) area: types types.d.ts, JSDoc annotations, hand-maintained declarations labels Aug 10, 2026
@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Generated code size

Comparing 15827d4 merged into 57f8dcf against 57f8dcf. Merging this PR will not change the code webpack generates.

Changed New Deleted Unchanged Raw change
Cases 0 0 0 1539 —
Assets 0 0 0 6007 —
Runtimes 0 0 0 1891 —

No asset changed size.

No runtime gained or lost a runtime module.

Built test/configCases with the defaults a user gets: 1539 case(s), 6007 asset(s), 40 emitted nothing.

`html-parser-document-unit` stops at the parse, so nothing measured
serialization — the half this branch changes. This covers both paths a node
can take (emitted in pieces as the walk reaches them, or composed from its
children and read back) and the tree shapes they cost differently on:
fan-out and depth are separate axes for a walk that carries one frame per
open element.

Against this branch's merge base every case separates well clear of its
spread, so a regression in either path shows up rather than hiding in the
document total:

  document           278.7 -> 68.8 ms
  fragment (tr)       50.6 -> 27.6 ms
  wide table          79.7 -> 35.9 ms
  flat siblings       21.9 -> 13.7 ms
  nested lists        30.3 -> 24.1 ms
  deep nesting        34.1 -> 25.1 ms
  composed subtrees   21.8 -> 18.2 ms
  implied start tags  13.2 -> 10.9 ms
@github-actions github-actions Bot added the area: build-perf Build time and memory of webpack itself label Aug 10, 2026
A build that is not minimizing still parses and walks, and that pass is the
floor the printer sits on. Running every fixture both ways makes the pair the
measurement: the difference is what printing costs on that shape, which
neither number shows alone.

  fixture              walk    minify
  document            25.6 ->  61.2 ms
  fragment (tr)       13.4 ->  27.9 ms
  wide table          15.4 ->  32.8 ms
  flat siblings        6.7 ->  13.6 ms
  nested lists        10.8 ->  23.5 ms
  deep nesting        12.7 ->  26.9 ms
  composed subtrees    7.2 ->  15.8 ms
  implied start tags   3.9 ->   9.9 ms

The fixtures move to a table so the two modes cannot drift apart; the
`minify (...)` names are unchanged, so their history carries over.
@alexander-akait
alexander-akait marked this pull request as ready for review August 10, 2026 06:07

@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: 3

🧹 Nitpick comments (3)
test/benchmarkCases/html-printer-document-unit/index.bench.mjs (1)

143-158: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

The benchmark discards output, so it cannot catch a printing regression.

Both registrations throw the result away. The PR claims piecewise printing is byte-identical to the composed path, and this file is the only place both paths run over large realistic documents. A one-time equality check outside the timed loop would turn that claim into evidence, at no cost to the measurement.

The repository guidance also asks for deterministic counts or retained-heap evidence rather than wall-clock alone. The PR's headline claim is a peak-memory reduction, which timing does not show.

♻️ Proposed addition
 export default (bench) => {
 	for (const [name, source, options] of FIXTURES) {
+		// Outside the timed loop: the piecewise printer must agree with the
+		// composed one, which timing alone would never show.
+		const printed = new SourceProcessor().process(source, {
+			...options,
+			minimize: true
+		}).code;
+		if (typeof printed !== "string" || printed.length === 0) {
+			throw new Error(`html-printer-document-unit: ${name} printed nothing`);
+		}
 		bench.add(
 			`unit benchmark "html-printer-document-unit", walk (${name})`,
 			() => {
 				new SourceProcessor().process(source, options);
 			}
 		);

As per coding guidelines: "For performance or memory claims, prefer deterministic counts, profiling, or retained-heap evidence over wall-clock timing; use real emitting builds and verify module counts, emitted-file hashes, and diagnostics remain unchanged."

🤖 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/benchmarkCases/html-printer-document-unit/index.bench.mjs` around lines
143 - 158, Update the benchmark registrations in the default export to retain
and compare the composed and piecewise processing outputs once per fixture
outside the timed callbacks, asserting byte-identical results while keeping the
callbacks focused on measurement. Add deterministic evidence for the claimed
peak-memory reduction, such as retained-heap or allocation measurements, and
preserve the existing large-document benchmark coverage.

Source: Coding guidelines

test/HtmlSyntax.unittest.js (1)

4334-4340: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fixtures are hand-sized against a private threshold they cannot read. _STREAM_MIN_NODES is module-private in lib/html/syntax.js, so every fixture that must cross it is sized by a manual estimate and a comment. Raise the threshold and the streamed path stops running in all three places, with no test failure and no benchmark signal. Export the constant and derive the fixture sizes from it.

  • test/HtmlSyntax.unittest.js#L4334-L4340: derive BIG and BIG_ROWS from the exported _STREAM_MIN_NODES instead of the literals 40000 and 15000, and drop the hard-coded "49152 nodes" from the comment.
  • test/HtmlSyntax.unittest.js#L4527-L4574: the offsets document is far below the threshold, so it never streams. Add a variant whose body crosses the derived threshold and assert the same head element spans.
  • test/benchmarkCases/html-printer-document-unit/index.bench.mjs#L54-L137: size each generator so it is unambiguously above or below the derived threshold, and label which path each fixture measures.
🤖 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/HtmlSyntax.unittest.js` around lines 4334 - 4340, Export the
module-private _STREAM_MIN_NODES constant from lib/html/syntax.js and use it to
derive BIG and BIG_ROWS in test/HtmlSyntax.unittest.js at lines 4334-4340,
removing the hard-coded 49152-node reference from the comment. In
test/HtmlSyntax.unittest.js lines 4527-4574, add an offsets-document variant
whose body exceeds the derived threshold and assert the same head-element spans.
In test/benchmarkCases/html-printer-document-unit/index.bench.mjs lines 54-137,
size every generator clearly above or below _STREAM_MIN_NODES and label the
streaming versus non-streaming path each fixture measures.
lib/html/syntax.js (1)

8026-8036: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Guard the streamed-cap invariant. Keep _STREAM_MIN_NODES + _FLUSH_BATCH * 2 below _COLUMN_SHRINK_CAPACITY. If the threshold becomes smaller than the current cap (57344), streamed parses that reach the cap release their columns and reallocate them on the next parse. Add an invariant check near these constants.

🤖 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/html/syntax.js` around lines 8026 - 8036, Add an invariant check near the
stream threshold and column capacity constants to ensure _STREAM_MIN_NODES +
_FLUSH_BATCH * 2 remains below _COLUMN_SHRINK_CAPACITY. The check should fail
immediately when configuration changes make the streamed cap equal to or exceed
the shrink capacity, preserving the existing cap logic in the _streamEligible
path.
🤖 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/html/syntax.js`:
- Around line 5619-5647: Move the JSDoc block describing popping the
open-element stack so it directly precedes popOpen, preserving its `@returns`
{HtmlElement} declaration. Keep removeOpenAt documented only by its own
removal-oriented JSDoc, so each helper has the correct generated type
information.
- Around line 9727-9761: Document that the HTML processing flow rooted at
grammar is non-reentrant because its module-level walk state cannot preserve an
outer traversal when visitors recursively call SourceProcessor.process(). Add
the constraint to the relevant public-facing documentation or comments,
explicitly stating that nested HTML processing is unsupported; do not alter the
separate CSS processing behavior.

In `@test/HtmlSyntax.unittest.js`:
- Around line 4527-4574: Update the “reports final end offsets for head
elements” test to force SourceProcessor’s streamed-walk path by expanding the
body with enough nodes to exceed _STREAM_MIN_NODES, while preserving the
existing head markup. Assert the same final title, script, style, and link spans
and element ordering so the test fails when streaming reports provisional head
end offsets.

---

Nitpick comments:
In `@lib/html/syntax.js`:
- Around line 8026-8036: Add an invariant check near the stream threshold and
column capacity constants to ensure _STREAM_MIN_NODES + _FLUSH_BATCH * 2 remains
below _COLUMN_SHRINK_CAPACITY. The check should fail immediately when
configuration changes make the streamed cap equal to or exceed the shrink
capacity, preserving the existing cap logic in the _streamEligible path.

In `@test/benchmarkCases/html-printer-document-unit/index.bench.mjs`:
- Around line 143-158: Update the benchmark registrations in the default export
to retain and compare the composed and piecewise processing outputs once per
fixture outside the timed callbacks, asserting byte-identical results while
keeping the callbacks focused on measurement. Add deterministic evidence for the
claimed peak-memory reduction, such as retained-heap or allocation measurements,
and preserve the existing large-document benchmark coverage.

In `@test/HtmlSyntax.unittest.js`:
- Around line 4334-4340: Export the module-private _STREAM_MIN_NODES constant
from lib/html/syntax.js and use it to derive BIG and BIG_ROWS in
test/HtmlSyntax.unittest.js at lines 4334-4340, removing the hard-coded
49152-node reference from the comment. In test/HtmlSyntax.unittest.js lines
4527-4574, add an offsets-document variant whose body exceeds the derived
threshold and assert the same head-element spans. In
test/benchmarkCases/html-printer-document-unit/index.bench.mjs lines 54-137,
size every generator clearly above or below _STREAM_MIN_NODES and label the
streaming versus non-streaming path each fixture measures.
🪄 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: e278bd81-6a93-4db2-927b-010637ff4191

📥 Commits

Reviewing files that changed from the base of the PR and between 5c5d009 and abe7208.

⛔ Files ignored due to path filters (1)
  • types.d.ts is excluded by !types.d.ts
📒 Files selected for processing (5)
  • lib/html/HtmlGenerator.js
  • lib/html/syntax.js
  • lib/util/SourceProcessor.js
  • test/HtmlSyntax.unittest.js
  • test/benchmarkCases/html-printer-document-unit/index.bench.mjs
🔗 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/html/syntax.js Resolved
Comment thread lib/html/syntax.js Resolved
Comment thread test/HtmlSyntax.unittest.js Resolved
`PrintOptions.mode` has always been typed `"minify" | "beautify"` and both
printers branch on it, but `process()` hardcoded `"minify"`, so half of it
was unreachable. `process(src, { mode })` now selects it; `minimize: true`
keeps working as the shorthand for `mode: "minify"` it reads as.

Beautifying re-serializes rather than rewrites: the source's own attribute
quoting, its comments and its end tags all come back, and nothing is dropped
for re-parsing the same. It is deliberately still ugly — nothing is
re-indented — but it must not be lossy, and on the CSS side two things were:
kept license comments were only collected when minifying, and a custom
property's value claimed them from the writer only on the minifying path, so
each was emitted twice. Both are fixed here.

Minify output is unchanged, byte-for-byte, over the 843-file css/html
fixture corpus and 24,000 random inputs. On the same inputs
`minify(beautify(x)) === minify(x)` holds everywhere minify is itself
idempotent.

Each printer benchmark fixture now runs in all three modes, so
`minify - beautify` shows what the rewrites cost on top of serializing:

  fixture              walk  beautify  minify
  document            26.4 ->   48.2 ->  57.8 ms
  fragment (tr)       12.3 ->   22.1 ->  25.1 ms
  wide table          14.1 ->   25.4 ->  34.3 ms
  flat siblings        6.4 ->   12.9 ->  14.8 ms
  nested lists        10.8 ->   17.5 ->  23.9 ms
  deep nesting        12.3 ->   17.5 ->  23.9 ms
  composed subtrees    7.1 ->   13.9 ->  16.3 ms
  implied start tags   3.9 ->    7.5 ->   9.6 ms
@github-actions github-actions Bot added the area: css CSS support (lib/css) label Aug 10, 2026
Three review findings on the walk streaming:

- The JSDoc describing `popOpen` sat above `removeOpenAt`, so one helper was
  documented by a block declaring a return value it does not have and the
  other had none. Moved onto the function it describes.

- Hoisting the walk's state (`_visitors`, `_writer`, the open path, the AST
  columns) to module scope for monomorphic call sites made the grammar
  non-reentrant: a visitor that started another HTML parse would take this
  one's state over and release its columns on the way out, silently. It used
  to work, because the state was per-call. Refuse it with a clear error
  instead. Nesting the other language is untouched — a `<style>`'s CSS has
  its own module state — and an `<iframe srcdoc>` is a separate module
  webpack parses on its own, which is why every `configCases/html` case
  still passes.

- The offsets test was a few dozen nodes against a 49152-node threshold, so
  it never reached the streamed walk it described, and its comment credited
  the wrong mechanism: head elements do not keep final offsets because
  streaming starts in body, but because a flush only ever visits children
  that are already complete, and `HtmlParser` reads `tagEnd()` — assigned at
  insertion — for the still-open `<html>` / `<head>`. Replaced with the
  contrast that does pin the streamed path: the same markup below and above
  the threshold, asserting an entered element reports a provisional `end` at
  `enter` and a final one at `exit`. Verified it bites — pushing the
  threshold out of reach fails it, where the old test passed.

Output is unchanged: the fuzz corpus hashes identically with and without
these commits.
@github-actions

Copy link
Copy Markdown
Contributor

Types Coverage

Coverage after merging perf/html-stream-source-processor-walk into main will be
99.27%
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.12%100%100%99.12%214, 234
   CodeGenerationResults.js100%100%100%100%
   CompatibilityPlugin.js100%100%100%100%
   Compilation.js98.43%100%100%98.43%1673, 1992, 1999, 2007, 2029, 2032, 2971, 3450–3451, 3483, 4192, 4222, 4275–4276, 4280, 4285, 4301–4302, 4316–4317, 4322–4323, 4843, 4869, 549, 554, 5679, 5711, 5728, 5746, 5762, 5777, 5802–5803, 5805, 6139, 6144, 6150, 6153, 6160, 6172, 6174, 6178, 6196, 6211, 6245, 6301, 6325, 6441, 801–802
   Compiler.js99.56%100%100%99.56%1168–1169, 1177
   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%1220, 1223, 519–523, 525, 671
   ExternalModuleFactoryPlugin.js100%100%100%100%
   ExternalsPlugin.js100%100%100%100%
   FileSystemInfo.js99.53%100%100%99.53%186, 2454–2455, 2458, 2469, 2480, 2491, 284, 3928, 3943, 3967
   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%721
   MultiStats.js100%100%100%100%
   MultiWatching.js100%100%100%100%
   NoEmitOnErrorsPlugin.js100%100%100%100%
   NodeStuffPlugin.js100%100%100%100%
   NormalModule.js97.99%100%100%97.99%1027, 1030, 1047, 1064, 1312, 1346, 1362, 1817, 2114, 2119–2129, 29
   NormalModuleFactory.js98.77%100%100%98.77%1142, 1410, 1421, 1431, 1482–1484, 1491, 536, 548
   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%690
   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.js95.70%100%100%95.70%319, 375, 384, 387, 411, 429, 450–451, 474, 494–495, 531–532, 555, 568–569, 641, 654, 675, 694
   RuntimeTemplate.js99.90%100%100%99.90%119
   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%376
   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.12%100%100%97.12%10, 267, 289, 291
lib/asset
   AssetBytesGenerator.js100%100%100%100%
   AssetBytesParser.js100%100%100%100%
   AssetGenerator.js100%100%100%100%
   AssetModule.js100%100%100%100%
   AssetModulesPlugin.js98.03%100%100%98.03%324, 348, 351, 43, 48, 481
   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,

@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.

🧹 Nitpick comments (3)
lib/html/syntax.js (1)

9793-9798: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten the source comments.

Both comments exceed the two-line limit. Keep only the non-obvious contract.

  • lib/html/syntax.js#L9793-L9798: State that HTML processing is non-reentrant because walk state is module-scoped.
  • lib/html/syntax.js#L5688-L5693: State that popOpen notifies the streamed walk. Preserve the @returns annotation.

As per coding guidelines: “Keep comments as short as possible, generally one line and no more than two short lines; comments must add non-obvious information and must not narrate or restate code.”

🤖 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/html/syntax.js` around lines 9793 - 9798, Shorten the comment at
lib/html/syntax.js lines 9793-9798 to state only that HTML processing is
non-reentrant because walk state is module-scoped. Also shorten the comment at
lib/html/syntax.js lines 5688-5693 to state that popOpen notifies the streamed
walk, while preserving its `@returns` annotation.

Source: Coding guidelines

test/HtmlSyntax.unittest.js (2)

5416-5428: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Name the failing source in the round-trip loop.

The loop runs five sources through one expect. On failure Jest reports only the differing strings, not which input produced them. it.each or an array-shaped assertion makes the failing case identifiable, matching the pattern already used at lines 5751-5759.

♻️ Label each case
-		for (const source of [
+		for (const source of [
 			'<!DOCTYPE html><div id=a class="x"><!-- c --><p>hi</p></div>',
 			"<table><tr><td>x</table>",
 			"<div><pre>\n\nkeep</pre><template><p>t</p></template></div>",
 			"<svg><path/></svg><math><mi>x</mi></math>",
 			"<p><b>a<p>b"
 		]) {
-			expect(print(print(source, "beautify"), "minify")).toBe(
-				print(source, "minify")
-			);
+			expect([source, print(print(source, "beautify"), "minify")]).toEqual([
+				source,
+				print(source, "minify")
+			]);
 		}
🤖 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/HtmlSyntax.unittest.js` around lines 5416 - 5428, Update the round-trip
test in the “beautifies to something that minifies back the same” case to label
each source input, using it.each or an equivalent array-shaped assertion as in
the nearby pattern. Preserve the existing five inputs and comparison behavior
while ensuring failures identify the specific source.

4612-4613: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin the small-document offsets to a value, not to each other.

expect(smallEnter).toBe(smallExit) passes when both stay -1. If the div visitor never fires — a renamed hook, a changed default, a skipped element — the assertion still succeeds and the streamed/non-streamed contrast is lost. The big case is protected because it asserts absolute lengths.

♻️ Assert the expected span
 		const [smallEnter, smallExit] = ends(10);
-		expect(smallEnter).toBe(smallExit);
+		expect(smallExit).toBe(
+			`<div id=outer>${"<p><b>x</b></p>".repeat(10)}</div>`.length
+		);
+		expect(smallEnter).toBe(smallExit);
🤖 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/HtmlSyntax.unittest.js` around lines 4612 - 4613, Update the
small-document assertion in ends(10) to compare smallEnter and smallExit against
the expected absolute offset value, rather than only comparing them to each
other. Keep the existing streamed/non-streamed span check while ensuring the
assertion fails if the div visitor does not fire or defaults remain unchanged.
🤖 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.

Nitpick comments:
In `@lib/html/syntax.js`:
- Around line 9793-9798: Shorten the comment at lib/html/syntax.js lines
9793-9798 to state only that HTML processing is non-reentrant because walk state
is module-scoped. Also shorten the comment at lib/html/syntax.js lines 5688-5693
to state that popOpen notifies the streamed walk, while preserving its `@returns`
annotation.

In `@test/HtmlSyntax.unittest.js`:
- Around line 5416-5428: Update the round-trip test in the “beautifies to
something that minifies back the same” case to label each source input, using
it.each or an equivalent array-shaped assertion as in the nearby pattern.
Preserve the existing five inputs and comparison behavior while ensuring
failures identify the specific source.
- Around line 4612-4613: Update the small-document assertion in ends(10) to
compare smallEnter and smallExit against the expected absolute offset value,
rather than only comparing them to each other. Keep the existing
streamed/non-streamed span check while ensuring the assertion fails if the div
visitor does not fire or defaults remain unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f4b10f90-ac1b-4dd4-ae18-7aff8b921ec6

📥 Commits

Reviewing files that changed from the base of the PR and between abe7208 and 15827d4.

⛔ Files ignored due to path filters (1)
  • types.d.ts is excluded by !types.d.ts
📒 Files selected for processing (5)
  • lib/html/HtmlGenerator.js
  • lib/html/syntax.js
  • lib/util/SourceProcessor.js
  • test/HtmlSyntax.unittest.js
  • test/benchmarkCases/html-printer-document-unit/index.bench.mjs
🔗 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 (3)
  • lib/util/SourceProcessor.js
  • test/benchmarkCases/html-printer-document-unit/index.bench.mjs
  • lib/html/HtmlGenerator.js

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: html Experimental HTML support (lib/html) 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