Repository navigation
perf: load webpack's own modules only when a build needs them - #21652
Conversation
🦋 Changeset detectedLatest commit: 99ca00b 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 |
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Lazy loading foundations and runtime lifecycle lib/util/lazyModule.js, lib/util/preloadModuleType.js, lib/NormalModuleFactory.js, lib/Compilation.js, types.d.ts |
Adds lazy module access, module-type preparation, deferred runtime-module attachment, runtime-module hashing and generation, and public API declarations. |
Runtime-module integrations lib/RuntimePlugin.js, lib/css/*, lib/asset/*, lib/esm/*, lib/prefetch/*, lib/runtime/*, lib/wasm-*/*, lib/web/*, lib/webworker/* |
Runtime modules, parsers, and generators now load only when required. Module-type preparation preloads implementations before synchronous hooks use them. |
Shared infrastructure and dependency updates
| Layer / File(s) | Summary |
|---|---|
Shared hooks and syntax utilities lib/hmr/*, lib/HotModuleReplacementPlugin.js, lib/container/*, lib/javascript/syntax.js, lib/prefetch/*, lib/util/globUtils.js |
Adds shared HMR and Module Federation hook registries, static CommonJS require scanning, resource-hint classification, and direct plugin-hook access. |
Lazy errors and platform integrations lib/Compiler.js, lib/NormalModule.js, lib/FileSystemInfo.js, lib/node/*, lib/util/*, lib/dependencies/*, lib/url/*, lib/schemes/*, lib/sharing/* |
Defers loading of warnings, errors, watchers, MIME tables, serialization components, network modules, and parser dependencies. Selected direct imports replace cycle-prone lazy accessors. |
Validation
| Layer / File(s) | Summary |
|---|---|
Integration and unit tests test/lazyModule.unittest.js, test/collectCjsRequireSpecifiers.unittest.js, test/HotModuleReplacementPlugin.unittest.js, test/configCases/plugins/*, test/HtmlEntryWrapper.unittest.js |
Tests lazy-loader caching, CommonJS scanning, HMR hook identity, deferred runtime-module errors and attachment, module-type preparation, and HTML entry wrapping. |
Release metadata .changeset/510-lazy-runtime-module-loading.md |
Adds a minor changeset for on-demand runtime-module, parser, and generator loading. |
Suggested labels: area: build-perf
🚥 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 performance change, but the branch prefix is not provided for verification. | Provide the branch name or confirm that its 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.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
lib/HotModuleReplacementPlugin.js (1)
113-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInternal call now bypasses the static method.
createAcceptHandlercalls the module function directly. Before, the call went throughHotModuleReplacementPlugin.getParserHooks. Both resolve to the same implementation, so behavior is unchanged today.The difference matters only if a downstream tool replaces the static property to intercept hook creation. That interception no longer reaches internal callers. If preserving that seam is not a goal, keep the direct call.
🤖 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/HotModuleReplacementPlugin.js` at line 113, The internal call in createAcceptHandler should preserve the HotModuleReplacementPlugin.getParserHooks override seam by routing hook creation through the static method rather than invoking the module-level function directly; update only this call and keep the existing behavior otherwise unchanged.lib/FileSystemInfo.js (1)
2249-2264: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
addedset is now redundant.
collectCjsRequireSpecifiersreturns aSet, so each specifier already appears once. Theaddedset at Line 2248 and theadded.has(...)check at Line 2254 can be removed. This is a small cleanup only.🤖 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/FileSystemInfo.js` around lines 2249 - 2264, Remove the redundant added set and its added.has(dependency) guard from the loop over collectCjsRequireSpecifiers in the CJS dependency handling flow; retain the existing node: and builtinModules filtering and dependency push behavior.lib/hmr/parserHooks.js (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidation style differs from
createHooksRegistry.
lib/util/createHooksRegistry.jswas changed in this same PR to replaceinstanceofwith a constructor-name check, so a compilation from another webpack copy passes. This module keepsinstanceof JavascriptParser, so a parser from another webpack copy is rejected.The behavior matches what
HotModuleReplacementPlugindid before, so this is not a regression. Consider aligning the two checks if cross-copy support is intended for parsers as well. The eagerrequireofJavascriptParserexists only to support this check; a name check would also remove that import.Also applies to: 33-37
🤖 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/hmr/parserHooks.js` at line 9, Align the parser validation in the relevant hook registration logic with createHooksRegistry by replacing the instanceof JavascriptParser check with the established constructor-name check. Remove the now-unused eager JavascriptParser require while preserving rejection of non-parser values and acceptance of parsers from other webpack copies.test/collectCjsRequireSpecifiers.unittest.js (2)
92-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo case names describe behavior the assertions do not check.
The case at Line 92 is named "template specifier is static", and the case at Line 94 is named "template specifier with substitution". In both,
require(\./a`)produces an empty set from the Acorn reference, because the argument is aTemplateLiteraland not aLiteral`. The scanner also produces an empty set. The test therefore asserts that neither collects a template specifier, which is the opposite of "is static".Rename to state the checked behavior, for example "template specifier is not collected".
🤖 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/collectCjsRequireSpecifiers.unittest.js` around lines 92 - 96, Rename the two test cases in the collectCjsRequireSpecifiers suite so their names accurately state that template-literal require specifiers are not collected, including both the static template and substituted template cases. Leave the test inputs and assertions unchanged.
125-125: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the file-count floor with a looser value.
toBeGreaterThan(400)couples the test to the current size oflib/. The guard exists to catch a broken directory walk, and a much lower floor serves that purpose without needing an update whenever the tree shrinks.♻️ Proposed change
- expect(files.length).toBeGreaterThan(400); + expect(files.length).toBeGreaterThan(100);🤖 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/collectCjsRequireSpecifiers.unittest.js` at line 125, Lower the file-count floor in the test assertion using toBeGreaterThan so it still detects a broken lib directory walk without coupling the test to the current file count; leave the surrounding collection behavior unchanged.test/HotModuleReplacementPlugin.unittest.js (1)
6-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an assertion that pins the delegation to the shared module.
This PR moves hook storage into
lib/hmr/parserHooks.js. The tests confirm the static method still works, but nothing confirms both entry points share oneWeakMap. If a future change givesHotModuleReplacementPluginits own registry again, these tests still pass while plugins that call the module directly get different hooks.💚 Proposed additional assertion
it("attaches the hooks once per parser", () => { const parser = new JavascriptParser(); const hooks = HotModuleReplacementPlugin.getParserHooks(parser); expect(typeof hooks.hotAcceptCallback.tap).toBe("function"); expect(typeof hooks.hotAcceptWithoutCallback.tap).toBe("function"); expect(HotModuleReplacementPlugin.getParserHooks(parser)).toBe(hooks); + expect(require("../lib/hmr/parserHooks")(parser)).toBe(hooks); expect( HotModuleReplacementPlugin.getParserHooks(new JavascriptParser()) ).not.toBe(hooks); });As per coding guidelines: "Cover every changed line and branch with tests".
🤖 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/HotModuleReplacementPlugin.unittest.js` around lines 6 - 17, Extend the “attaches the hooks once per parser” test to import or reference the shared parser-hooks module and assert that HotModuleReplacementPlugin.getParserHooks(parser) returns the same hooks as the module’s direct entry point for that parser, confirming both APIs use the same WeakMap-backed registry.Source: Coding guidelines
lib/node/nodeConsole.js (1)
9-10: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winKeep the CLI dependency lazy on the custom-console path.
NodeEnvironmentPlugincan useinfrastructureLogging.consolewithout callingnodeConsole. The top-levelrequire("../cli")still loads the CLI module whennodeConsole.jsis loaded. This removes the previous lazy boundary on a supported path.Keep the require inside the factory unless the eager load is intentional and measured for this performance PR.
Suggested fix
-const Cli = require("../cli"); +const memoize = require("../util/memoize"); +const getCli = memoize(() => require("../cli")); ... - const c = Cli.createColors({ useColor: Boolean(colors) }); + const c = getCli().createColors({ useColor: Boolean(colors) });Also applies to: 60-60
🤖 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/node/nodeConsole.js` around lines 9 - 10, Keep the CLI dependency lazy by removing the top-level require of “../cli” and loading it inside the nodeConsole factory only when the CLI-backed console path is used. Preserve the existing custom-console behavior and avoid changing the unrelated truncateArgs import.lib/Compilation.js (1)
4418-4423: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDrain the queue until it is empty.
addRuntimeModulecalls theruntimeModulehook. A tap on that hook can calladdLazyRuntimeModuleagain, which refills_pendingRuntimeModulesafter this method cleared it. Those entries are never attached, and the runtime module is dropped without an error.Repeat the load-and-attach pass while the queue is non-empty, or throw when the queue is refilled after the pass.
🤖 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/Compilation.js` around lines 4418 - 4423, Update the pending runtime-module processing around addRuntimeModule so it drains _pendingRuntimeModules completely: after each load-and-attach pass, detect entries added by the runtimeModule hook and repeat processing until the queue is empty, or explicitly throw if refilling is unsupported. Ensure every module queued through addLazyRuntimeModule is attached before invoking callback.test/lazyModule.unittest.js (1)
6-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest concurrent lazy-module access.
Lines 13-15 await the first load before the second call. This does not verify that concurrent
get()calls share one in-flight load. Callget()twice before awaiting either result. Assert one loader call and identical resolved values.As per coding guidelines,
test/**/*.{js,json}requires coverage of every changed line and branch.Proposed test update
- const first = await get(); + const firstPromise = get(); + const secondPromise = get(); + const first = await firstPromise; expect(first).toEqual({ answer: 42 }); - expect(await get()).toBe(first); + expect(await secondPromise).toBe(first); expect(calls).toBe(1);🤖 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/lazyModule.unittest.js` around lines 6 - 17, Update the lazy-module test around lazyModule and get() to start two get() calls before awaiting either result, then await both and assert the loader runs once and both results are the same resolved module object. Preserve the initial zero-call assertion and existing module-value verification while covering concurrent in-flight access.Source: Coding guidelines
🤖 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 @.changeset/510-lazy-runtime-module-loading.md:
- Around line 5-6: Rewrite the changeset description as one concise imperative
sentence of no more than 80 characters, ending with a period, while preserving
the core lazy runtime module loading change.
In `@lib/Compilation.js`:
- Around line 4414-4425: Update _attachPendingRuntimeModules so errors thrown by
create(loaded[i]) or addRuntimeModule during the fulfillment handler are routed
to callback, by attaching the error handler to the promise chain after the
attach loop rather than only passing it to the initial then call.
- Around line 6067-6069: Restore this.chunkGraph to its original value
immediately after processRuntimeRequirements returns and before calling
_attachPendingRuntimeModules. Keep the deferred callback responsible only for
handling the error and invoking the existing continuation, since the attach
logic uses its captured graph and does not require this.chunkGraph to remain
swapped.
In `@lib/html/HtmlModulesPlugin.js`:
- Line 439: Update the head-tag construction in HtmlModulesPlugin to use the
eagerly loaded HtmlParser module reference when calling buildHeadTags, rather
than invoking getHtmlParser.loaded() synchronously. Preserve the existing
htmlObj argument and ensure configurations with output.html and JavaScript
entries can build head tags before compilation.
In `@lib/javascript/syntax.js`:
- Around line 7174-7189: Update lib/javascript/syntax.js lines 7174-7189 in
startsRegExp by adding "await" to KEYWORDS_BEFORE_REGEXP, and document the
existing ) and ] limitation in its JSDoc without adding paren-context tracking.
Update test/collectCjsRequireSpecifiers.unittest.js lines 80-86 with an async
await-before-regexp case containing require("./nope") plus the existing trailing
require assertion; do not add a control-flow-`)` regexp case unless that
position is implemented.
In `@lib/NormalModuleFactory.js`:
- Around line 1536-1542: In the hook.callAsync completion handler, notify all
callbacks in the waiting loop before invoking the primary callback, so a
synchronous throw from callback cannot prevent queued factorize flows from
completing. Preserve the existing error normalization and module-type
preparation bookkeeping, and ensure every waiting callback is invoked once.
In `@lib/schemes/HttpUriPlugin.js`:
- Line 528: Update the validation diagnostic name in HttpUriPlugin to use "Http
Uri Plugin" instead of "getHttp() Uri Plugin", preserving the correct component
label and avoiding exposure of the internal loader name.
In `@lib/webpack.js`:
- Around line 14-15: Remove the eager top-level requires for MultiCompiler and
ProgressPlugin in lib/webpack.js, and restore memoized lazy getters for each
module or constructor. Resolve MultiCompiler only in the multi-compiler
conditional path and ProgressPlugin only when progress is enabled, preserving
cached reuse after the first lookup.
---
Nitpick comments:
In `@lib/Compilation.js`:
- Around line 4418-4423: Update the pending runtime-module processing around
addRuntimeModule so it drains _pendingRuntimeModules completely: after each
load-and-attach pass, detect entries added by the runtimeModule hook and repeat
processing until the queue is empty, or explicitly throw if refilling is
unsupported. Ensure every module queued through addLazyRuntimeModule is attached
before invoking callback.
In `@lib/FileSystemInfo.js`:
- Around line 2249-2264: Remove the redundant added set and its
added.has(dependency) guard from the loop over collectCjsRequireSpecifiers in
the CJS dependency handling flow; retain the existing node: and builtinModules
filtering and dependency push behavior.
In `@lib/hmr/parserHooks.js`:
- Line 9: Align the parser validation in the relevant hook registration logic
with createHooksRegistry by replacing the instanceof JavascriptParser check with
the established constructor-name check. Remove the now-unused eager
JavascriptParser require while preserving rejection of non-parser values and
acceptance of parsers from other webpack copies.
In `@lib/HotModuleReplacementPlugin.js`:
- Line 113: The internal call in createAcceptHandler should preserve the
HotModuleReplacementPlugin.getParserHooks override seam by routing hook creation
through the static method rather than invoking the module-level function
directly; update only this call and keep the existing behavior otherwise
unchanged.
In `@lib/node/nodeConsole.js`:
- Around line 9-10: Keep the CLI dependency lazy by removing the top-level
require of “../cli” and loading it inside the nodeConsole factory only when the
CLI-backed console path is used. Preserve the existing custom-console behavior
and avoid changing the unrelated truncateArgs import.
In `@test/collectCjsRequireSpecifiers.unittest.js`:
- Around line 92-96: Rename the two test cases in the
collectCjsRequireSpecifiers suite so their names accurately state that
template-literal require specifiers are not collected, including both the static
template and substituted template cases. Leave the test inputs and assertions
unchanged.
- Line 125: Lower the file-count floor in the test assertion using
toBeGreaterThan so it still detects a broken lib directory walk without coupling
the test to the current file count; leave the surrounding collection behavior
unchanged.
In `@test/HotModuleReplacementPlugin.unittest.js`:
- Around line 6-17: Extend the “attaches the hooks once per parser” test to
import or reference the shared parser-hooks module and assert that
HotModuleReplacementPlugin.getParserHooks(parser) returns the same hooks as the
module’s direct entry point for that parser, confirming both APIs use the same
WeakMap-backed registry.
In `@test/lazyModule.unittest.js`:
- Around line 6-17: Update the lazy-module test around lazyModule and get() to
start two get() calls before awaiting either result, then await both and assert
the loader runs once and both results are the same resolved module object.
Preserve the initial zero-call assertion and existing module-value verification
while covering concurrent in-flight access.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 603a0549-c0d5-476c-8d8d-fce45f284a60
📒 Files selected for processing (92)
.changeset/510-lazy-runtime-module-loading.mdlib/ChunkTemplate.jslib/Compilation.jslib/Compiler.jslib/ExternalModule.jslib/FileSystemInfo.jslib/HotModuleReplacementPlugin.jslib/MainTemplate.jslib/ModuleTemplate.jslib/MultiCompiler.jslib/NodeStuffPlugin.jslib/NormalModule.jslib/NormalModuleFactory.jslib/ProgressPlugin.jslib/RuntimePlugin.jslib/RuntimeTemplate.jslib/asset/AssetModulesPlugin.jslib/buildChunkGraph.jslib/config/target.jslib/container/ContainerPlugin.jslib/container/HoistContainerReferencesPlugin.jslib/container/ModuleFederationPlugin.jslib/container/moduleFederationHooks.jslib/css/CssGenerator.jslib/css/CssModulesPlugin.jslib/css/CssParser.jslib/dependencies/AMDRequireDependenciesBlockParserPlugin.jslib/dependencies/CommonJsFullRequireDependency.jslib/dependencies/CommonJsImportsParserPlugin.jslib/dependencies/CommonJsRequireDependency.jslib/dependencies/ContextDependency.jslib/dependencies/HarmonyDetectionParserPlugin.jslib/dependencies/HarmonyEvaluatedImportSpecifierDependency.jslib/dependencies/HarmonyImportDependencyParserPlugin.jslib/dependencies/HarmonyImportGuard.jslib/dependencies/HarmonyImportSpecifierDependency.jslib/dependencies/ImportDependency.jslib/dependencies/ImportMetaGlobHelpers.jslib/dependencies/ImportParserPlugin.jslib/dependencies/RequireHeaderDependency.jslib/dependencies/WorkerAndWorkletPlugin.jslib/esm/ModuleChunkLoadingPlugin.jslib/hmr/parserHooks.jslib/html/HtmlGenerator.jslib/html/HtmlModulesPlugin.jslib/html/HtmlParser.jslib/javascript/JavascriptParserHelpers.jslib/javascript/syntax.jslib/loaders/loadLoader.jslib/node/NodeEnvironmentPlugin.jslib/node/NodeWatchFileSystem.jslib/node/nodeConsole.jslib/optimize/ConcatenatedModule.jslib/optimize/SideEffectsFlagPlugin.jslib/prefetch/ChunkPrefetchPreloadPlugin.jslib/prefetch/ResourceHintPlugin.jslib/prefetch/ResourceHintRuntimeModule.jslib/prefetch/parseResourceHintOptions.jslib/runtime/StartupChunkDependenciesPlugin.jslib/schemes/HttpUriPlugin.jslib/schemes/VirtualUrlPlugin.jslib/sharing/ConsumeSharedPlugin.jslib/sharing/resolveMatchedConfigs.jslib/typescript/TypeScriptPlugin.jslib/url/URLParserPlugin.jslib/util/createHooksRegistry.jslib/util/globUtils.jslib/util/lazyModule.jslib/util/mimeTypes.jslib/util/parseJson.jslib/util/preloadModuleType.jslib/util/serialization.jslib/wasm-async/AsyncWebAssemblyModulesPlugin.jslib/wasm-async/AsyncWebAssemblyParser.jslib/wasm-sync/WebAssemblyModulesPlugin.jslib/web/JsonpChunkLoadingPlugin.jslib/webpack.jslib/webworker/ImportScriptsChunkLoadingPlugin.jstest/Compilation.unittest.jstest/HotModuleReplacementPlugin.unittest.jstest/collectCjsRequireSpecifiers.unittest.jstest/configCases/plugins/prepare-module-type-error/errors.jstest/configCases/plugins/prepare-module-type-error/file.customtest/configCases/plugins/prepare-module-type-error/index.jstest/configCases/plugins/prepare-module-type-error/webpack.config.jstest/configCases/plugins/prepare-module-type/first.customtest/configCases/plugins/prepare-module-type/index.jstest/configCases/plugins/prepare-module-type/second.customtest/configCases/plugins/prepare-module-type/warnings.jstest/configCases/plugins/prepare-module-type/webpack.config.jstest/lazyModule.unittest.jstypes.d.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
webpack/schema-utils(auto-detected)webpack/tapable(auto-detected)
| this.options, | ||
| { | ||
| name: "Http Uri Plugin", | ||
| name: "getHttp() Uri Plugin", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restore the HttpUriPlugin name in the validation diagnostic.
The new "getHttp() Uri Plugin" label exposes an internal loader name and identifies the wrong component. Restore "Http Uri Plugin".
Proposed fix
- name: "getHttp() Uri Plugin",
+ name: "Http Uri Plugin",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: "getHttp() Uri Plugin", | |
| name: "Http Uri Plugin", |
🤖 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/schemes/HttpUriPlugin.js` at line 528, Update the validation diagnostic
name in HttpUriPlugin to use "Http Uri Plugin" instead of "getHttp() Uri
Plugin", preserving the correct component label and avoiding exposure of the
internal loader name.
1b08795 to
ac6c912
Compare
|
This PR is packaged and the instant preview is available (8e28763). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@8e28763
yarn add -D webpack@https://pkg.pr.new/webpack@8e28763
pnpm add -D webpack@https://pkg.pr.new/webpack@8e28763 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #21652 +/- ##
==========================================
+ Coverage 94.40% 94.46% +0.06%
==========================================
Files 628 629 +1
Lines 80618 80967 +349
Branches 23812 23889 +77
==========================================
+ Hits 76108 76487 +379
+ Misses 4510 4480 -30
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:
|
Generated code sizeComparing
9 asset(s) changed size
1 runtime(s) changed which runtime modules they carry
Built |
Defer parsers, generators, runtime modules and error classes behind `lazyModule`/`memoize` so a build only loads what it reaches. Adds `Compilation#addLazyRuntimeModule` and the `NormalModuleFactory` hook `prepareModuleType`, which awaits a type's loads at the factory's async boundary because `createParser`/`createGenerator` are synchronous. Replaces the acorn walk in `FileSystemInfo` with a CommonJS specifier scanner.
The runtime module read the chunk's modules through an unordered cache, so the emitted order was whichever comparator happened to sort them last. Deferring runtime module attachment changes that, flipping the mapping.
The scanner reads whatever is on disk, so it must terminate on input the acorn reference cannot parse — those branches need direct assertions rather than the differential comparison.
Main removed test/Compilation.unittest.js while converting unit tests to config cases; these drive the same two paths through a real build.
d53906c to
96eff54
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/FileSystemInfo.js`:
- Around line 2247-2249: Update the dependency collection around
collectCjsRequireSpecifiers so escaped static CommonJS string literals, such as
require("./a\\tb"), are retained and decoded correctly. Prefer fixing the shared
scanner used by collectCjsRequireSpecifiers; otherwise preserve the Acorn
fallback for this path, while keeping non-static or invalid specifiers excluded.
In `@lib/html/headTags.js`:
- Around line 42-47: Shorten the comment above the head-tag serialization logic
to no more than two brief lines, retaining only the non-obvious spec ordering
and the distinction between synthetic entries and authored pages.
In `@test/HtmlEntryWrapper.unittest.js`:
- Around line 9-12: Shorten the comment above the synthetic page setup to two
short lines, retaining only the non-obvious requirement that this is the first
build and no lazy loads have been warmed.
🪄 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: 50438b88-caf8-4292-8160-20ccb7327a92
⛔ Files ignored due to path filters (1)
types.d.tsis excluded by!types.d.ts
📒 Files selected for processing (97)
.changeset/510-lazy-runtime-module-loading.mdlib/ChunkTemplate.jslib/Compilation.jslib/Compiler.jslib/ExternalModule.jslib/FileSystemInfo.jslib/HotModuleReplacementPlugin.jslib/MainTemplate.jslib/ModuleTemplate.jslib/MultiCompiler.jslib/NodeStuffPlugin.jslib/NormalModule.jslib/NormalModuleFactory.jslib/ProgressPlugin.jslib/RuntimePlugin.jslib/RuntimeTemplate.jslib/asset/AssetModulesPlugin.jslib/buildChunkGraph.jslib/config/target.jslib/container/ContainerPlugin.jslib/container/HoistContainerReferencesPlugin.jslib/container/ModuleFederationPlugin.jslib/container/RemoteRuntimeModule.jslib/container/moduleFederationHooks.jslib/css/CssGenerator.jslib/css/CssModulesPlugin.jslib/css/CssParser.jslib/dependencies/AMDRequireDependenciesBlockParserPlugin.jslib/dependencies/CommonJsFullRequireDependency.jslib/dependencies/CommonJsImportsParserPlugin.jslib/dependencies/CommonJsRequireDependency.jslib/dependencies/ContextDependency.jslib/dependencies/HarmonyDetectionParserPlugin.jslib/dependencies/HarmonyEvaluatedImportSpecifierDependency.jslib/dependencies/HarmonyImportDependencyParserPlugin.jslib/dependencies/HarmonyImportGuard.jslib/dependencies/HarmonyImportSpecifierDependency.jslib/dependencies/ImportDependency.jslib/dependencies/ImportMetaGlobHelpers.jslib/dependencies/ImportParserPlugin.jslib/dependencies/RequireHeaderDependency.jslib/dependencies/WorkerAndWorkletPlugin.jslib/esm/ModuleChunkLoadingPlugin.jslib/hmr/parserHooks.jslib/html/HtmlGenerator.jslib/html/HtmlModulesPlugin.jslib/html/HtmlParser.jslib/html/headTags.jslib/javascript/JavascriptParserHelpers.jslib/javascript/syntax.jslib/loaders/loadLoader.jslib/node/NodeWatchFileSystem.jslib/optimize/ConcatenatedModule.jslib/optimize/SideEffectsFlagPlugin.jslib/prefetch/ChunkPrefetchPreloadPlugin.jslib/prefetch/ResourceHintPlugin.jslib/prefetch/ResourceHintRuntimeModule.jslib/prefetch/parseResourceHintOptions.jslib/runtime/StartupChunkDependenciesPlugin.jslib/schemes/HttpUriPlugin.jslib/schemes/VirtualUrlPlugin.jslib/sharing/ConsumeSharedPlugin.jslib/sharing/resolveMatchedConfigs.jslib/typescript/TypeScriptPlugin.jslib/url/URLParserPlugin.jslib/util/createHooksRegistry.jslib/util/globUtils.jslib/util/lazyModule.jslib/util/mimeTypes.jslib/util/parseJson.jslib/util/preloadModuleType.jslib/util/serialization.jslib/wasm-async/AsyncWebAssemblyModulesPlugin.jslib/wasm-async/AsyncWebAssemblyParser.jslib/wasm-sync/WebAssemblyModulesPlugin.jslib/web/JsonpChunkLoadingPlugin.jslib/webpack.jslib/webworker/ImportScriptsChunkLoadingPlugin.jstest/HotModuleReplacementPlugin.unittest.jstest/HtmlEntryWrapper.unittest.jstest/collectCjsRequireSpecifiers.unittest.jstest/configCases/plugins/add-lazy-runtime-module-error/errors.jstest/configCases/plugins/add-lazy-runtime-module-error/index.jstest/configCases/plugins/add-lazy-runtime-module-error/webpack.config.jstest/configCases/plugins/add-lazy-runtime-module/MarkerRuntimeModule.jstest/configCases/plugins/add-lazy-runtime-module/index.jstest/configCases/plugins/add-lazy-runtime-module/webpack.config.jstest/configCases/plugins/prepare-module-type-error/errors.jstest/configCases/plugins/prepare-module-type-error/file.customtest/configCases/plugins/prepare-module-type-error/index.jstest/configCases/plugins/prepare-module-type-error/webpack.config.jstest/configCases/plugins/prepare-module-type/first.customtest/configCases/plugins/prepare-module-type/index.jstest/configCases/plugins/prepare-module-type/second.customtest/configCases/plugins/prepare-module-type/warnings.jstest/configCases/plugins/prepare-module-type/webpack.config.jstest/lazyModule.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 (80)
- test/configCases/plugins/prepare-module-type/first.custom
- lib/dependencies/HarmonyEvaluatedImportSpecifierDependency.js
- lib/dependencies/HarmonyImportGuard.js
- test/configCases/plugins/prepare-module-type-error/errors.js
- test/configCases/plugins/prepare-module-type/warnings.js
- test/configCases/plugins/prepare-module-type-error/file.custom
- test/configCases/plugins/prepare-module-type/index.js
- lib/util/preloadModuleType.js
- lib/config/target.js
- test/lazyModule.unittest.js
- lib/html/HtmlModulesPlugin.js
- test/configCases/plugins/prepare-module-type/second.custom
- lib/javascript/JavascriptParserHelpers.js
- lib/sharing/resolveMatchedConfigs.js
- lib/util/parseJson.js
- lib/dependencies/ImportDependency.js
- lib/sharing/ConsumeSharedPlugin.js
- lib/wasm-async/AsyncWebAssemblyParser.js
- lib/web/JsonpChunkLoadingPlugin.js
- lib/dependencies/RequireHeaderDependency.js
- lib/util/globUtils.js
- lib/schemes/VirtualUrlPlugin.js
- lib/util/lazyModule.js
- lib/MainTemplate.js
- lib/dependencies/HarmonyDetectionParserPlugin.js
- lib/css/CssGenerator.js
- lib/webworker/ImportScriptsChunkLoadingPlugin.js
- lib/prefetch/ResourceHintPlugin.js
- lib/HotModuleReplacementPlugin.js
- lib/dependencies/ImportMetaGlobHelpers.js
- lib/dependencies/CommonJsRequireDependency.js
- lib/NodeStuffPlugin.js
- test/configCases/plugins/prepare-module-type-error/webpack.config.js
- lib/container/HoistContainerReferencesPlugin.js
- lib/buildChunkGraph.js
- lib/ModuleTemplate.js
- lib/dependencies/CommonJsFullRequireDependency.js
- lib/util/createHooksRegistry.js
- lib/prefetch/ResourceHintRuntimeModule.js
- lib/url/URLParserPlugin.js
- lib/hmr/parserHooks.js
- lib/Compiler.js
- lib/ProgressPlugin.js
- lib/dependencies/ContextDependency.js
- lib/container/ModuleFederationPlugin.js
- lib/container/ContainerPlugin.js
- lib/RuntimeTemplate.js
- lib/html/HtmlGenerator.js
- lib/prefetch/parseResourceHintOptions.js
- lib/dependencies/WorkerAndWorkletPlugin.js
- lib/runtime/StartupChunkDependenciesPlugin.js
- lib/prefetch/ChunkPrefetchPreloadPlugin.js
- lib/css/CssParser.js
- lib/optimize/ConcatenatedModule.js
- lib/javascript/syntax.js
- lib/wasm-sync/WebAssemblyModulesPlugin.js
- lib/NormalModuleFactory.js
- lib/ChunkTemplate.js
- lib/MultiCompiler.js
- lib/esm/ModuleChunkLoadingPlugin.js
- lib/dependencies/CommonJsImportsParserPlugin.js
- lib/css/CssModulesPlugin.js
- lib/dependencies/HarmonyImportSpecifierDependency.js
- lib/typescript/TypeScriptPlugin.js
- lib/NormalModule.js
- lib/asset/AssetModulesPlugin.js
- lib/schemes/HttpUriPlugin.js
- lib/wasm-async/AsyncWebAssemblyModulesPlugin.js
- lib/util/serialization.js
- lib/ExternalModule.js
- lib/html/HtmlParser.js
- lib/dependencies/HarmonyImportDependencyParserPlugin.js
- test/configCases/plugins/prepare-module-type/webpack.config.js
- lib/loaders/loadLoader.js
- lib/util/mimeTypes.js
- lib/Compilation.js
- lib/dependencies/AMDRequireDependenciesBlockParserPlugin.js
- lib/dependencies/ImportParserPlugin.js
- lib/RuntimePlugin.js
- test/configCases/plugins/prepare-module-type-error/index.js
| for (const dependency of collectCjsRequireSpecifiers( | ||
| source | ||
| )) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep escaped static CommonJS specifiers.
collectCjsRequireSpecifiers omits valid string literals with escapes. test/collectCjsRequireSpecifiers.unittest.js Lines 139-149 documents this behavior. On Bun, require("./a\\tb") is then absent from build dependencies, so changes to that module can miss cache invalidation.
Decode valid string escapes in the shared scanner, or retain the Acorn fallback for this path.
🤖 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/FileSystemInfo.js` around lines 2247 - 2249, Update the dependency
collection around collectCjsRequireSpecifiers so escaped static CommonJS string
literals, such as require("./a\\tb"), are retained and decoded correctly. Prefer
fixing the shared scanner used by collectCjsRequireSpecifiers; otherwise
preserve the Acorn fallback for this path, while keeping non-static or invalid
specifiers excluded.
| /** | ||
| * Serializes every head tag an `output.html` options object asks for, in | ||
| * spec order: charset, base, meta tags, title. Used where a page is created | ||
| * from scratch (HtmlModulesPlugin's synthetic entry wrapper); authored pages | ||
| * instead merge the options against their existing tags during `parse` — | ||
| * existing tags win. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Shorten this comment.
Keep the non-obvious behavior in at most two short lines.
As per coding guidelines, “Keep comments as short as possible, generally one line and no more than two short lines.” As per path instructions, “Comments 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/html/headTags.js` around lines 42 - 47, Shorten the comment above the
head-tag serialization logic to no more than two brief lines, retaining only the
non-obvious spec ordering and the distinction between synthetic entries and
authored pages.
Sources: Coding guidelines, Path instructions
The scanner rejected any literal holding a backslash, so a specifier written "..\\lib\\a" or hex-escaped by a minifier was dropped from Bun's build dependencies. Cook escapes the way the ast it replaces did.
…require Internal callers go back through HotModuleReplacementPlugin.getParserHooks, so replacing it still intercepts. The parser is matched by class name like createHooksRegistry does, which also stops parserHooks from loading JavascriptParser eagerly on every ESM build.
They only build strings out of escapeAttribute/escapeText, so they belong next to those rather than in a file of their own. Both callers already reach the syntax module, so this drops a require instead of adding one.
The marker wrote through `globalThis`, which Node 10 does not have, so the executed bundle threw there. Emit through RuntimeGlobals.global instead.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 4645-4647: Reduce the block comment above the head-tag
serialization logic to exactly two short prose lines, preserving the spec order
and the distinction that authored pages merge options during parse with their
own tags taking precedence.
🪄 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: 16757ea7-f930-44c0-b5a5-a7ec9b0f568f
⛔ Files ignored due to path filters (1)
types.d.tsis excluded by!types.d.ts
📒 Files selected for processing (12)
lib/HotModuleReplacementPlugin.jslib/hmr/parserHooks.jslib/html/HtmlModulesPlugin.jslib/html/HtmlParser.jslib/html/syntax.jslib/javascript/syntax.jstest/HotModuleReplacementPlugin.unittest.jstest/HtmlEntryWrapper.unittest.jstest/collectCjsRequireSpecifiers.unittest.jstest/configCases/plugins/add-lazy-runtime-module/MarkerRuntimeModule.jstest/configCases/plugins/add-lazy-runtime-module/index.jstest/configCases/plugins/add-lazy-runtime-module/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)
🚧 Files skipped from review as they are similar to previous changes (10)
- test/configCases/plugins/add-lazy-runtime-module/MarkerRuntimeModule.js
- test/configCases/plugins/add-lazy-runtime-module/index.js
- test/HotModuleReplacementPlugin.unittest.js
- test/collectCjsRequireSpecifiers.unittest.js
- lib/HotModuleReplacementPlugin.js
- lib/html/HtmlModulesPlugin.js
- lib/hmr/parserHooks.js
- lib/javascript/syntax.js
- test/configCases/plugins/add-lazy-runtime-module/webpack.config.js
- lib/html/HtmlParser.js
Types CoverageCoverage after merging perf/lazy-runtime-module-loading into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Summary
webpack(config)loaded and compiled 580 modules before any user code ran, including runtime modules a build never emits, the error hierarchy, watchpack,es-module-lexerand the browserslist/caniuse-lite tree — most builds use none of them. Deferring those to first use cuts a react production build by -4.3% instructions and -0.56MB setup heap (580 -> 504 modules), and by -5.0% / -1.42MB when the config names itstargetexplicitly (504 -> 483); emitted output is byte-identical across five fixtures in both modes. Two plugin-facing APIs make the deferral possible —Compilation#addLazyRuntimeModuleand theNormalModuleFactoryhookprepareModuleType— and four require cycles that the deferral was hiding are now broken outright, solib/has no static require cycle left.What kind of change does this PR introduce?
perf — it also adds the two plugin-facing APIs named above, which is why the changeset is a minor bump.
Did you add tests for your changes?
Yes —
test/lazyModule.unittest.js,test/collectCjsRequireSpecifiers.unittest.js(differential against the acorn AST over every file inlib/),test/HotModuleReplacementPlugin.unittest.js, newCompilation.unittest.jscases, andtest/configCases/plugins/prepare-module-type{,-error}.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?
Two additions for the plugin API docs:
Compilation#addLazyRuntimeModule(chunk, load, create)for runtime modules whose class should load only when emitted, andNormalModuleFactory.hooks.prepareModuleTypefor warming what a module type's synchronouscreateParser/createGeneratortaps read.Use of AI
AI (Claude) was used throughout: to survey deferral candidates, apply the mechanical conversions across ~90 files, and build the measurement harnesses. Every claim here is from a deterministic measurement I ran and checked (callgrind instruction counts,
used_heap_sizeafter forced GC, sha256 of emitted output); several AI-proposed changes were measured and rejected — notably alib/dependenciesrefactor that turned out to save nothing once base classes and template registrations were counted.Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes