fix: hash initial chunks before runtime chunks#20711
fix: hash initial chunks before runtime chunks#20711davidmurdoch wants to merge 4 commits intowebpack:mainfrom
Conversation
🦋 Changeset detectedLatest commit: f392dee 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 |
alexander-akait
left a comment
alexander-akait
left a comment
There was a problem hiding this comment.
Please use integration tests, we have watchCases and hotCases, unittests are useless
|
Is the watch case i added okay? |
| expect(currentChunkFile).not.toBe(STATE.previousChunkFile); | ||
| } | ||
|
|
||
| expect(serviceWorkerSource).toContain(currentChunkFile); |
There was a problem hiding this comment.
I think we don't need it to compare content, it is useless too, just test shared value, also test itself without errors and warnings means that we fixed the problem
There was a problem hiding this comment.
Just use import mod from "./shared.js" and expect(mod).toBe("__SHARED__") and in the next expect(mod).toBe("__SHARED__CHANGED")
There was a problem hiding this comment.
a simple test like:
import { sharedValue } from "./shared.js";
it("should update the runtime chunk filename map on rebuild", () => {
expect(sharedValue).toBe(
WATCH_STEP === "0" ? "__SHARED__" : "__SHARED__CHANGED"
);
});
does not catch the bug if the fix in this PR is reverted. The emitted service-worker.js content check is what actually fails before the fix and passes after.
There was a problem hiding this comment.
i've updated the test with a clarifying comment.
please feel free to edit the code yourself if you know a better way!
Merging this PR will improve performance by 25.82%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | benchmark "css-modules", scenario '{"name":"mode-production","mode":"production"}' |
8.5 MB | 6.8 MB | +25.82% |
Comparing davidmurdoch:fix/real-content-hash-watch-order (f392dee) with main (32a30c2)
There was a problem hiding this comment.
Pull request overview
This PR fixes a watch-mode caching bug where runtime chunks could embed stale filename/hash references by ensuring initial chunks are hashed before runtime chunks during Compilation.createHash().
Changes:
- Adjust chunk hashing order in
Compilation.createHash()to hash initial chunks before runtime chunks. - Add a new watch regression test case covering runtime chunk filename map updates across rebuilds.
- Add a changeset entry documenting the patch change.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/Compilation.js | Reorders chunk hashing so non-runtime (incl. initial) chunks are hashed before runtime chunks. |
| test/watchCases/cache/stale-runtime-chunk-filename-map/webpack.config.js | Adds a dedicated watch-mode repro config with splitChunks + memory cache to exercise runtime filename mapping updates. |
| test/watchCases/cache/stale-runtime-chunk-filename-map/test.config.js | Exposes a helper to read the emitted service worker asset for runtime-reference assertions. |
| test/watchCases/cache/stale-runtime-chunk-filename-map/0/service-worker.js | Asserts that the runtime output references the current hashed shared chunk filename on each watch step. |
| test/watchCases/cache/stale-runtime-chunk-filename-map/0/offscreen.js | Provides a second entry to force a shared initial chunk used by multiple entrypoints. |
| test/watchCases/cache/stale-runtime-chunk-filename-map/0/shared.js | Baseline shared module for step 0. |
| test/watchCases/cache/stale-runtime-chunk-filename-map/1/shared.js | Modified shared module for step 1 to force a rebuild and new chunk hash/filename. |
| .changeset/public-horses-visit.md | Records the behavior change as a patch-level release note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/cc @xiaoxiaojx Can you take a look at this, looks like something wrong with hashes, after changing of order, |
|
@xiaoxiaojx Yeah, you are right, maybe we can track circuital situations, no rush, but will be great to look at this |
|
Oh wow. That is tricky! Feel free to close this one, I don't mind. And ping me if you want to help testing solutions. |
Fixes #20710
Summary
Compilation.createHash()RealContentHashPluginreference errorTesting
node node_modules/eslint/bin/eslint.js lib/Compilation.js test/RealContentHashPlugin.test.jsyarn test:base --runInBand --runTestsByPath test/Watch.test.js test/RealContentHashPlugin.test.js