feat: skip require().prop in dead branches - #21517
#21517feat: skip require().prop in dead branches#21517avivkeller merged 1 commit intomainwebpack/webpack:mainfrom worktree-cjs-full-require-dead-branchwebpack/webpack:worktree-cjs-full-require-dead-branchCopy head branch name to clipboard
require().prop in dead branches#21517Conversation
🦋 Changeset detectedLatest commit: 422ae9b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This PR is packaged and the instant preview is available (075d0cd). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@075d0cd
yarn add -D webpack@https://pkg.pr.new/webpack@075d0cd
pnpm add -D webpack@https://pkg.pr.new/webpack@075d0cd |
Types CoverageCoverage after merging worktree-cjs-full-require-dead-branch into main will be
Coverage Report |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21517 +/- ##
==========================================
+ Coverage 93.50% 93.78% +0.27%
==========================================
Files 619 620 +1
Lines 72956 73719 +763
Branches 21000 21264 +264
==========================================
+ Hits 68221 69139 +918
+ Misses 4735 4580 -155
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:
|
Merging this PR will degrade performance by 0.12%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "many-modules-commonjs", scenario '{"name":"mode-production","mode":"production"}' |
7.7 MB | 10.1 MB | -23.37% |
| ❌ | Memory | benchmark "cache-filesystem", scenario '{"name":"mode-production","mode":"production"}' |
3.9 MB | 4.9 MB | -20.38% |
| ⚡ | Memory | benchmark "concatenate-modules", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
2.5 MB | 1.9 MB | +30.04% |
| ⚡ | Memory | benchmark "many-chunks-commonjs", scenario '{"name":"mode-production","mode":"production"}' |
9.3 MB | 7.4 MB | +25.44% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing worktree-cjs-full-require-dead-branch (422ae9b) with main (9d01716)
Summary
require()andimport()calls in provably dead conditional branches are already skipped (#21136), but arequire("./x").propmember chain was not —CommonJsFullRequireDependencycarried no branch guards, so its module stayed in the bundle. This attaches the guard stack to that dependency and emitsnull /* dead branch */when the connection is inactive. Refs #21136.What kind of change does this PR introduce?
feat
Did you add tests for your changes?
Yes — new cases in
test/configCases/inline-exports/cross-module-dead-branch/coveringrequire().prop,require().prop(), nested branches, and a control case with a non-static test.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
Claude Code was used to locate the existing guard infrastructure, write the dependency/parser changes and the test cases, and run the targeted test suites; all output was reviewed by me.
Note
Medium Risk
Touches CommonJS dependency graph conditions and codegen; behavior is aligned with existing
require()dead-branch logic but affects bundle membership and emitted member chains.Overview
Extends dead-branch elimination for inlined imported constants so
require("./m").prop(andrequire().prop()) behave like plainrequire(): modules only referenced on provably dead paths are dropped from the graph.CommonJsFullRequireDependencynow storesbranchGuards, implementsgetConditionviaHarmonyImportGuard.isDeadByGuards, serializes guards, and in the template replaces the full member chain withnull /* dead branch */when the module connection is inactive.CommonJsImportsParserPlugincallsattachDependencyGuardsfor member-chain and call-member-chain handlers;HarmonyImportGuardtreats full require deps as guardable.Config-case tests cover ternary,
if, nested branches,require().prop(), and a non-static test that still keeps the module.Reviewed by Cursor Bugbot for commit 422ae9b. Bugbot is set up for automated code reviews on this repo. Configure here.