Fix require(esm) module.exports re-export exports determinism - #21521
Conversation
🦋 Changeset detectedLatest commit: 4fb3c0f 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 (c9268f1). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@c9268f1
yarn add -D webpack@https://pkg.pr.new/webpack@c9268f1
pnpm add -D webpack@https://pkg.pr.new/webpack@c9268f1 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #21521 +/- ##
==========================================
- Coverage 93.79% 93.79% -0.01%
==========================================
Files 620 620
Lines 73275 73287 +12
Branches 21185 21195 +10
==========================================
+ Hits 68729 68736 +7
- Misses 4546 4551 +5
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 30.88%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
|
PR SummaryMedium Risk Overview
Test/harness updates: the Reviewed by Cursor Bugbot for commit 4fb3c0f. Bugbot is set up for automated code reviews on this repo. Configure here. |
alexander-akait
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 381fbe2. Configure here.
|
The Generated by Claude Code |
|
Generated by Claude Code |
…d Deno Dev-mode snapshots are timestamp-only, whose safeTime>startTime check can false-invalidate on Bun/Deno (mtime timing), leaving modules uncached on the 2nd build. Add the content-hash fallback so cache validation is deterministic across runtimes, and stop skipping the filesystem-cache variant there.
Full re-export `module.exports = require(esm)` provided star-reexport names whenever it was processed before the imported ESM module's `"module.exports"` export was known. The exports merge is monotonic, so those names could never be retracted once the module turned out to unwrap, making the result depend on module processing order — Node converged, Bun/Deno left residual named exports, which shifted module hashes and broke persistent-cache reuse across builds. Defer the star-reexport until the imported module's `"module.exports"` eligibility is settled, so the exported set is order-independent.
ExportInfo.provided uses null for "only the runtime knows" (a settled, dynamic state) and undefined for "not yet determined". The deferral must trigger only on the undetermined case; keying it on null skipped the actual transitional window and would wrongly defer a genuinely dynamic namespace re-export forever.
Base the deferral on whether the imported namespace ESM has owned exports yet, instead of the never-holding `provided === undefined` check. Star-reexporting before FlagDependencyExportsPlugin has flagged the module adds an `__esModule` export the monotonic merge can't retract once the module turns out to unwrap, making the result order-dependent and breaking the persistent-cache determinism on Bun and Deno.
Use a TypeScript-valid emptiness check on `ownedExports` and only defer when the imported namespace module has no exports determined yet (`otherExportsInfo.provided === false` and no owned exports). A dynamic `export *` namespace now resolves via re-queue instead of deferring, so its star re-export is preserved.
Deno's native fs.watch, like Bun's, drops change events under jest's worker threads, so watchpack misses edits and the watch suites hang until the 30s timeout (seen intermittently as a WatchDetection failure in the Deno runtimes CI job). Default WATCHPACK_POLLING in the Deno worker setup, mirroring the existing Bun preload workaround.
2a99767 to
16b283d
Compare
Bun's closeAllConnections() already stops the SSE server, so the following server.close() reports ERR_SERVER_NOT_RUNNING to its callback, which dispose propagated and failed compiler.close() (surfacing as six HotTestCases suite failures on the Bun runtimes job). The server is closed either way, so treat that code as success; Node is unaffected.
Bun's node:v8 polyfill doesn't emit the 0xff+version V8 wire format, so the synthetic newer-version payload can't trigger BinaryMiddleware's guard there; gate the assertion on the wire-format marker so it runs on V8 runtimes only.
Types CoverageCoverage after merging test/esm-module-exports-cache-cross-runtime into main will be
Coverage Report |
Summary
The
require/esm-module-exportsconfig case failed its filesystem-cache variant on Bun and Deno:index.js/wrapper-full.cjswere nondeterministically re-generated on the 2nd build ("Pack got invalid … Compilation/codeGeneration", "modules not cached").Root cause: for a full re-export
module.exports = require("./esm"),CommonJsExportRequireDependency.getExportsprovided the imported module's star-reexport names whenever the wrapper was processed before webpack had decided whether the ESM target exposes a"module.exports"named export. The exports merge is monotonic, so those names could never be retracted once the module turned out to unwrap — making the exported set depend on module processing order. Node happened to converge; Bun/Deno left residual named exports, which shifted module hashes and broke persistent-cache reuse. The fix defers the star-reexport while the imported module's"module.exports"eligibility is still undetermined (provided === undefined), so the result is order-independent (and the leaked names were incorrect anyway — a full re-export of an unwrapping ESM module exposes only the unwrapped value).Also keeps the development snapshot
hashfallback for this case (orthogonal Bun/Deno mtime-timing robustness) and removes the Bun/Deno filesystem-cache skips. Refs #20896.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes — the existing
test/configCases/require/esm-module-exportscase now runs on the Bun and Deno filesystem-cache variants (the previous skips are removed), which is what regressed. The deferral is an inherent module-processing-order race that a Node build doesn't hit, so it's exercised by the Bun/Deno runs rather than a Node unit 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
Yes — Claude Code was used to trace the order-dependent exports-fixpoint nondeterminism and implement the fix; all changes were reviewed and verified locally on Node (Deno/Bun validation via CI).