Repository navigation
test(html): cover parser tree-construction edge cases and generator fallbacks - #21631
Conversation
…ate insertion-mode reset Eleven tree-construction and tokenizer lines codecov reports as never run, reached through a real build: the three CDATA section-end paths and EOF in one, the `</p>` break-out and any-other-end-tag walk in foreign content (with the MathML/SVG arms of isSpecial), and resetting the insertion mode to the cell or caption a template closes in.
|
322f083 to
3fd509c
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21631 +/- ##
==========================================
+ Coverage 94.23% 94.40% +0.17%
==========================================
Files 628 628
Lines 79952 80618 +666
Branches 23676 23812 +136
==========================================
+ Hits 75344 76110 +766
+ Misses 4608 4508 -100
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… regexes from the inline assertions
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe pull request adds HTML regression tests for HMR concatenation, inline chunk hash resolution, SVG foreign-content parsing, table foster parenting, and template insertion modes. ChangesHTML regression tests
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/configCases/html/concatenation-hmr-bailout/webpack.config.js (1)
19-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten this test comment.
This comment uses three long lines. Keep the HMR rationale in no more than two short lines.
Proposed wording
- // HMR wires self-accept against a per-module `module.hot`, so an HTML - // module merged into a shared scope would patch the wrong module id — - // the generator has to refuse concatenation for it. + // HMR uses per-module `module.hot`. + // Keep HTML modules out of concatenation.As per coding guidelines: comments in
{lib,hot,tooling,test}/**/*.{js,mjs}should be as short as possible, generally one line and no more than two short lines, and must add non-obvious information without narrating or restating code.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b979c9c-a74d-4af8-9008-16b7f5777506
⛔ Files ignored due to path filters (6)
test/configCases/html/parser-foreign-content/__snapshots__/ConfigCacheTest.snapis excluded by!**/*.snaptest/configCases/html/parser-foreign-content/__snapshots__/ConfigTest.snapis excluded by!**/*.snaptest/configCases/html/parser-foster-parenting/__snapshots__/ConfigCacheTest.snapis excluded by!**/*.snaptest/configCases/html/parser-foster-parenting/__snapshots__/ConfigTest.snapis excluded by!**/*.snaptest/configCases/html/parser-template-insertion-mode/__snapshots__/ConfigCacheTest.snapis excluded by!**/*.snaptest/configCases/html/parser-template-insertion-mode/__snapshots__/ConfigTest.snapis excluded by!**/*.snap
📒 Files selected for processing (20)
test/configCases/html/concatenation-hmr-bailout/index.jstest/configCases/html/concatenation-hmr-bailout/page.htmltest/configCases/html/concatenation-hmr-bailout/webpack.config.jstest/configCases/html/inline-chunk-hash/src.jstest/configCases/html/inline-chunk-hash/style.csstest/configCases/html/inline-chunk-hash/test.config.jstest/configCases/html/inline-chunk-hash/test.jstest/configCases/html/inline-chunk-hash/webpack.config.jstest/configCases/html/parser-foreign-content/index.jstest/configCases/html/parser-foreign-content/page.htmltest/configCases/html/parser-foreign-content/test.config.jstest/configCases/html/parser-foreign-content/webpack.config.jstest/configCases/html/parser-foster-parenting/index.jstest/configCases/html/parser-foster-parenting/page.htmltest/configCases/html/parser-foster-parenting/test.config.jstest/configCases/html/parser-foster-parenting/webpack.config.jstest/configCases/html/parser-template-insertion-mode/index.jstest/configCases/html/parser-template-insertion-mode/page.htmltest/configCases/html/parser-template-insertion-mode/test.config.jstest/configCases/html/parser-template-insertion-mode/webpack.config.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
webpack/schema-utils(auto-detected)webpack/tapable(auto-detected)
| it("should keep an HTML module out of concatenation when HMR is enabled", () => { | ||
| expect(html).toContain("<p>hmr page</p>"); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target test ---'
cat -n test/configCases/html/concatenation-hmr-bailout/index.js
printf '%s\n' '--- case files ---'
find test/configCases/html/concatenation-hmr-bailout -maxdepth 2 -type f -print | sort
printf '%s\n' '--- related case references ---'
rg -n --glob '*.js' --glob '*.json' 'concatenation-hmr-bailout|hmr page|HTML module|concatenat' test/configCases/html lib test | head -200
printf '%s\n' '--- current diff summary ---'
git diff --stat
git diff -- test/configCases/html/concatenation-hmr-bailoutRepository: webpack/webpack
Length of output: 23018
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- case configuration and fixture ---'
cat -n test/configCases/html/concatenation-hmr-bailout/webpack.config.js
cat -n test/configCases/html/concatenation-hmr-bailout/page.html
printf '%s\n' '--- HtmlGenerator bailout implementation ---'
sed -n '1050,1105p' lib/html/HtmlGenerator.js
sed -n '1325,1360p' lib/html/HtmlGenerator.js
printf '%s\n' '--- Module API and concatenation consumers ---'
sed -n '1125,1165p' lib/Module.js
rg -n --glob '*.js' 'getConcatenationBailoutReason|concatenationBailout|bailoutReason|ConcatenatedModule' lib test/configCases | head -160
printf '%s\n' '--- config-case tests that inspect compilation/module shape ---'
rg -n --glob 'index.js' 'compilation|module.*concaten|concatenat.*module|__webpack_modules__|webpackChunk|moduleIds|stats' test/configCases | head -200Repository: webpack/webpack
Length of output: 41252
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- HTML concatenation tests ---'
cat -n test/configCases/html/concatenation/index.js
cat -n test/configCases/html/concatenation/webpack.config.js
printf '%s\n' '--- representative stats/module-shape assertions ---'
cat -n test/configCases/concatenate-modules/commonjs-require-shapes/index.js | sed -n '1,90p'
cat -n test/configCases/defer-import/concatenated-require-wrapped/index.js | sed -n '1,45p'
cat -n test/configCases/concatenate-modules/top-level-declarations/webpack.config.js | sed -n '1,80p'
printf '%s\n' '--- test globals and stats setup ---'
rg -n --glob '*.js' '__STATS__|__STATS_I__|stats.*modules|modules.*filter' test/configCases test | head -160
printf '%s\n' '--- concatenation decision path ---'
sed -n '95,235p' lib/optimize/ModuleConcatenationPlugin.js
sed -n '450,500p' lib/optimize/ModuleConcatenationPlugin.js
printf '%s\n' '--- relevant test-runner documentation/config ---'
rg -n 'configCases|__STATS__|stats' test | head -120Repository: webpack/webpack
Length of output: 42865
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
generator = Path("lib/html/HtmlGenerator.js").read_text()
config = Path("test/configCases/html/concatenation-hmr-bailout/webpack.config.js").read_text()
test = Path("test/configCases/html/concatenation-hmr-bailout/index.js").read_text()
checks = {
"HMR plugin enabled": "new HotModuleReplacementPlugin()" in config,
"module concatenation enabled": "concatenateModules: true" in config,
"bailout checks module.hot": "if (module.hot)" in generator,
"bailout returns a reason": 'return "HTML module needs its own module scope for HMR";' in generator,
"concatenation path requires no HMR": "generateContext.concatenationScope && !module.hot" in generator,
"test checks only exported HTML": 'expect(html).toContain("<p>hmr page</p>")' in test,
"test does not inspect compilation metadata": "__STATS__" not in test and "__webpack_modules__" not in test,
}
for name, value in checks.items():
print(f"{name}: {value}")
if not all(checks.values()):
raise SystemExit("unexpected source shape")
PYRepository: webpack/webpack
Length of output: 403
Assert the HMR concatenation bailout directly.
Use __STATS__.modules to assert that no concatenated module contains page.html. Keep the HTML content assertion as a separate output check.
Source: Coding guidelines
|
@alexander-akait unit is green now thanks :)) |
Merging this PR will degrade performance by 63.7%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "many-modules-esm", scenario '{"name":"mode-development","mode":"development"}' |
821.1 KB | 2,261.9 KB | -63.7% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing aryanraj45:test/html-parser-edge-cases (752f464) with main (5c5d009)
…ly the rendered content
Unable to generate the flame graphsThe performance report has correctly been generated, but there was an internal error while generating the flame graphs for this run. We're working on fixing the issue. Feel free to contact us on Discord or at [email protected] if the issue persists. |
|
This PR is packaged and the instant preview is available (57f8dcf). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@57f8dcf
yarn add -D webpack@https://pkg.pr.new/webpack@57f8dcf
pnpm add -D webpack@https://pkg.pr.new/webpack@57f8dcf |
Summary
Adds integration cases for fourteen lines in the experimental HTML parser and generator that the suite never reached. Each one is driven through a real build and verified with V8 coverage rather than by reading the code:
</p>break-out and any-other-end-tag walk in foreign content, including the MathML and SVG arms of the special-element checkNo production code changes.
A few neighbouring lines are deliberately left alone: two are marked in the source as unreachable from the spec paths, the name-intern collision branches depend on the built-in name lists (which have no hash collisions today) rather than on any input, and HTML modules never carry dependency blocks.
What kind of change does this PR introduce?
test
Did you add tests for your changes?
Yes — five cases under
test/configCases/html/:parser-foreign-content,parser-template-insertion-mode,parser-foster-parenting,concatenation-hmr-bailoutandinline-chunk-hash. The three parser cases snapshot the emitted page, since the tree the parser builds is only observable through the minifier's re-serialization.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
Partial
Note
Low Risk
Test-only additions under
test/configCases/html/; no runtime or bundler behavior is modified.Overview
Adds five integration cases under
test/configCases/html/to exercise previously uncovered experimental HTML parser and generator paths. No production code changes.Three parser scenarios snapshot the emitted
page.htmlafter minification re-serializes the parse tree: foreign content (CDATA end variants, EOF in CDATA,</p>break-out, foreign end-tag handling), template insertion mode reset when a<template>closes inside table caption/cell, and foster parenting for misplaced table content.Two generator-focused cases assert runtime/build behavior: concatenation + HMR keeps HTML modules out of module concatenation so
module.hotself-accept targets the correct module id, and inline chunk hash verifiesoutput.html.inlineresolves inlined chunks via chunk hash when no content hash is present (development), leaving no__WEBPACK_HTML_INLINE__sentinels inmain.html.Reviewed by Cursor Bugbot for commit 3fd509c. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit