Skip to content

Commit eab49ca

Browse files
authored
fix: scope parsed module sources by asset (#734)
* fix: scope parsed module sources by asset `getViewerData()` merged the parsed module map of every asset into a single flat object, but Webpack module IDs are only unique within a compilation. Assets produced by separate compilations - a main bundle and a worker bundle, or a multi-config build - both number their modules from `0`, so the last parsed asset overwrote the earlier ones and modules were attributed another asset's parsed source and size. Store parsed sources per asset name and look them up using only the current asset's map, so each module reports the source that was actually parsed out of the asset it belongs to. Fixes #732 * fix: isolate per-asset parsed source writes Scoping the parsed module maps by asset was not enough on its own. A module that belongs to several assets is represented by a single stats object, and every asset containing it receives that same object. The module tree keeps a live reference to it and reads `parsedSrc` lazily, after the per-asset loop has finished, so the asset processed last decided what all of them displayed. `Module.mergeData()` accumulated `size` and `parsedSrc` onto those shared objects for the same reason. Give each asset its own copies of its modules before attributing any parsed source, so writing a source for one asset can no longer change what another asset reports. `test/analyzerUtils.js` asserted on the stats objects it passed in, so it now asserts on the returned chart data instead, matching parsed size against the source length and pinning the source itself through its gzip size. * test: clarify shared-module fixture and coverage Replace the hand-written shared-module fixture with output from a documented Webpack 5 build. Include the source and configuration, use purpose-based bundle names, cover the stat-only fallback, and remove the unreachable array fallback.
1 parent a4d4123 commit eab49ca

12 files changed

Lines changed: 292 additions & 11 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack-bundle-analyzer": patch
3+
---
4+
5+
Scope parsed module sources to the asset they were parsed from, so assets that reuse the same module IDs, or that share a module, no longer report each other's parsed and compressed sizes.

‎src/analyzer.js‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -334,12 +334,14 @@ function getViewerData(bundleStats, bundleDir, opts) {
334334
// Trying to parse bundle assets and get real module sizes if `bundleDir` is provided
335335
/** @type {Record<string, { src: string, runtimeSrc: string }> | null} */
336336
let bundlesSources = null;
337-
/** @type {Record<string | number, boolean> | null} */
338-
let parsedModules = null;
337+
// Parsed sources are stored per asset because different assets can reuse the same module IDs
338+
// (e.g. a root bundle and a worker bundle both numbering their modules from `0`).
339+
/** @type {Record<string, import("./parseUtils").Modules> | null} */
340+
let parsedModulesByAsset = null;
339341

340342
if (bundleDir) {
341343
bundlesSources = {};
342-
parsedModules = {};
344+
parsedModulesByAsset = {};
343345

344346
for (const statAsset of bundleStats.assets) {
345347
const assetFile = path.join(bundleDir, statAsset.name);
@@ -380,12 +382,12 @@ function getViewerData(bundleStats, bundleDir, opts) {
380382
src: bundleInfo.src,
381383
runtimeSrc: bundleInfo.runtimeSrc,
382384
};
383-
Object.assign(parsedModules, bundleInfo.modules);
385+
parsedModulesByAsset[statAsset.name] = bundleInfo.modules;
384386
}
385387

386388
if (Object.keys(bundlesSources).length === 0) {
387389
bundlesSources = null;
388-
parsedModules = null;
390+
parsedModulesByAsset = null;
389391
logger.warn(
390392
"\nNo bundles were parsed. Analyzer will show only original module sizes from stats file.\n",
391393
);
@@ -415,13 +417,24 @@ function getViewerData(bundleStats, bundleDir, opts) {
415417
);
416418
}
417419

420+
// A module that is a part of more than one asset is one stats object. Every asset that
421+
// contains that module gets the same object. The module tree reads `parsedSrc` late, after
422+
// all assets are processed. Copy the modules for each asset. Then a parsed source that is
423+
// set for one asset cannot change what another asset shows.
424+
assetModules = assetModules.map((statsModule) => ({ ...statsModule }));
425+
418426
const asset = (result[statAsset.name] = /** @type {Asset} */ ({
419427
size: statAsset.size,
420428
}));
421429
const assetSources =
422430
bundlesSources && Object.hasOwn(bundlesSources, statAsset.name)
423431
? bundlesSources[statAsset.name]
424432
: null;
433+
const assetParsedModules =
434+
parsedModulesByAsset &&
435+
Object.hasOwn(parsedModulesByAsset, statAsset.name)
436+
? parsedModulesByAsset[statAsset.name]
437+
: null;
425438

426439
if (assetSources) {
427440
asset.parsedSize = Buffer.byteLength(assetSources.src);
@@ -440,16 +453,16 @@ function getViewerData(bundleStats, bundleDir, opts) {
440453
}
441454

442455
// Adding parsed sources
443-
if (parsedModules) {
456+
if (assetParsedModules) {
444457
/** @type {StatsModule[]} */
445458
const unparsedEntryModules = [];
446459

447460
for (const statsModule of assetModules) {
448461
if (
449462
typeof statsModule.id !== "undefined" &&
450-
parsedModules[statsModule.id]
463+
Object.hasOwn(assetParsedModules, statsModule.id)
451464
) {
452-
statsModule.parsedSrc = parsedModules[statsModule.id];
465+
statsModule.parsedSrc = assetParsedModules[statsModule.id];
453466
} else if (isEntryModule(statsModule)) {
454467
unparsedEntryModules.push(statsModule);
455468
}

‎test/analyzer.js‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ const fs = require("node:fs");
33
const path = require("node:path");
44
const url = require("node:url");
55
const puppeteer = require("puppeteer");
6+
const Logger = require("../src/Logger");
67
const { getViewerData } = require("../src/analyzer");
8+
const { parseBundle } = require("../src/parseUtils");
79
const { isZstdSupported } = require("../src/sizeUtils");
810

911
let browser;
@@ -126,6 +128,98 @@ describe("Analyzer", () => {
126128
});
127129
});
128130

131+
it("should not attribute parsed sources across assets that reuse module IDs", () => {
132+
const statsDir = path.resolve(
133+
__dirname,
134+
"./stats/with-worker-loader-dynamic-import",
135+
);
136+
// `getViewerData` changes the stats it receives, so read a new copy of them.
137+
const stats = JSON.parse(
138+
fs.readFileSync(path.join(statsDir, "stats.json"), "utf8"),
139+
);
140+
141+
// Two compilations number their modules from `0`. Module ID `0` is a different module in
142+
// each asset.
143+
const moduleId = "0";
144+
const rootSource = parseBundle(path.join(statsDir, "bundle.js")).modules[
145+
moduleId
146+
];
147+
const workerSource = parseBundle(path.join(statsDir, "bundle.worker.js"))
148+
.modules[moduleId];
149+
150+
expect(rootSource).toEqual(expect.any(String));
151+
expect(workerSource).not.toBe(rootSource);
152+
153+
const chartData = getViewerData(stats, statsDir);
154+
const rootAsset = chartData.find((asset) => asset.label === "bundle.js");
155+
156+
// `parsedSize` is the length of the parsed source.
157+
expect(rootAsset.groups).toHaveLength(1);
158+
expect(rootAsset.groups[0].parsedSize).toBe(rootSource.length);
159+
});
160+
161+
it("should not attribute parsed sources across assets that share a module", () => {
162+
const statsDir = path.resolve(
163+
__dirname,
164+
"./stats/with-module-in-multiple-assets",
165+
);
166+
// `getViewerData` changes the stats it receives, so read a new copy of them.
167+
const stats = JSON.parse(
168+
fs.readFileSync(path.join(statsDir, "stats.json"), "utf8"),
169+
);
170+
171+
// `messages.js` is one source module used by two bundles. Each bundle uses a different
172+
// export, so Webpack creates a different factory for the module in each bundle.
173+
const sharedModuleId = "906";
174+
const expectedParsedSizes = {
175+
"long-message.js": parseBundle(path.join(statsDir, "long-message.js"))
176+
.modules[sharedModuleId].length,
177+
"short-message.js": parseBundle(path.join(statsDir, "short-message.js"))
178+
.modules[sharedModuleId].length,
179+
};
180+
181+
expect(expectedParsedSizes["long-message.js"]).not.toBe(
182+
expectedParsedSizes["short-message.js"],
183+
);
184+
185+
const chartData = getViewerData(stats, statsDir);
186+
const parsedSizes = Object.fromEntries(
187+
chartData.map((asset) => [
188+
asset.label,
189+
asset.groups[0].groups.find((group) => group.label === "messages.js")
190+
.parsedSize,
191+
]),
192+
);
193+
194+
expect(parsedSizes).toEqual(expectedParsedSizes);
195+
});
196+
197+
it("should show only stat sizes when no bundle can be parsed", () => {
198+
const statsDir = path.resolve(
199+
__dirname,
200+
"./stats/with-module-in-multiple-assets",
201+
);
202+
const stats = JSON.parse(
203+
fs.readFileSync(path.join(statsDir, "stats.json"), "utf8"),
204+
);
205+
const logger = new Logger("silent");
206+
const warn = jest.spyOn(logger, "warn");
207+
208+
// No asset file is in this directory, so every asset fails to parse.
209+
const chartData = getViewerData(stats, path.join(statsDir, "src"), {
210+
logger,
211+
});
212+
213+
expect(warn).toHaveBeenCalledWith(
214+
expect.stringContaining("No bundles were parsed"),
215+
);
216+
217+
for (const asset of chartData) {
218+
expect(asset.parsedSize).toBeUndefined();
219+
expect(asset.statSize).toBeGreaterThan(0);
220+
}
221+
});
222+
129223
it("should update the treemap when a chunk is deselected", async () => {
130224
generateReportFrom("with-worker-loader-dynamic-import/stats.json");
131225
const page = await browser.newPage();

‎test/analyzerUtils.js‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ const fs = require("node:fs");
22
const path = require("node:path");
33

44
const { getViewerData } = require("../src/analyzer");
5+
const { getCompressedSize } = require("../src/sizeUtils");
56

67
const BUNDLES_DIR = path.resolve(__dirname, "./bundles");
78

@@ -76,10 +77,22 @@ describe("getViewerData", () => {
7677
},
7778
};
7879

79-
getViewerData(stats, BUNDLES_DIR);
80+
const chartData = getViewerData(stats, BUNDLES_DIR);
81+
const modulesByPath = Object.fromEntries(
82+
chartData[0].groups[0].groups.map((group) => [group.path, group]),
83+
);
8084

81-
expect(dependencyModule.parsedSrc).toBe(expectedModules[447]);
82-
expect(entryModule.parsedSrc).toBe(expectedModules[956]);
85+
// Asserting on the returned chart data rather than on the input stats objects, because
86+
// `getViewerData` copies the modules of every asset before attributing parsed sources to them.
87+
// Module `parsedSize` is the source length, and the gzip size pins the source itself.
88+
expect(modulesByPath["./src/dependency.js"]).toMatchObject({
89+
parsedSize: expectedModules[447].length,
90+
gzipSize: getCompressedSize("gzip", expectedModules[447]),
91+
});
92+
expect(modulesByPath["./src/entry.js"]).toMatchObject({
93+
parsedSize: expectedModules[956].length,
94+
gzipSize: getCompressedSize("gzip", expectedModules[956]),
95+
});
8396
expect(chunksAccessCount).toBe(1);
8497
});
8598
});
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Fixture: one module in two assets
2+
3+
This fixture tests one source module that Webpack puts in two output bundles.
4+
5+
## Test case
6+
7+
The build has two entry files:
8+
9+
- `src/long-message-entry.js`
10+
- `src/short-message-entry.js`
11+
12+
Both files import `src/messages.js`. This is the source module that the two bundles share.
13+
14+
`messages.js` exports a long message and a short message. Each entry file imports only one of
15+
these exports. Webpack creates these bundles:
16+
17+
- `long-message.js` contains the long message.
18+
- `short-message.js` contains the short message.
19+
20+
The configuration disables module concatenation, split chunks, and the separate runtime chunk.
21+
These settings keep the module visible in both bundles.
22+
23+
## Stats and bundle sizes
24+
25+
Webpack gives `messages.js` module ID `906`. The stats report its source size as `87` in both
26+
chunks.
27+
28+
Webpack removes the unused export from each bundle. This makes the emitted module factories
29+
different:
30+
31+
- The factory in `long-message.js` has 59 characters.
32+
- The factory in `short-message.js` has 37 characters.
33+
34+
The analyzer must report the correct factory size for each bundle.
35+
36+
## Rebuild
37+
38+
Run this command from the repository root:
39+
40+
```sh
41+
npx webpack --config test/stats/with-module-in-multiple-assets/webpack.config.js
42+
```
43+
44+
The `stats.json` file is a reduced copy of the Webpack stats. It contains only the fields that the
45+
analyzer reads.

‎test/stats/with-module-in-multiple-assets/long-message.js‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/stats/with-module-in-multiple-assets/short-message.js‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { longMessage } from "./messages.js";
2+
3+
console.log(longMessage);
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export const longMessage = "This message is longer.";
2+
export const shortMessage = "x";
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { shortMessage } from "./messages.js";
2+
3+
console.log(shortMessage);

0 commit comments

Comments
 (0)