diff --git a/.changeset/nervous-workers-attribute.md b/.changeset/nervous-workers-attribute.md new file mode 100644 index 00000000..47f6485d --- /dev/null +++ b/.changeset/nervous-workers-attribute.md @@ -0,0 +1,5 @@ +--- +"webpack-bundle-analyzer": patch +--- + +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. diff --git a/src/analyzer.js b/src/analyzer.js index 076fd6a1..63954c62 100644 --- a/src/analyzer.js +++ b/src/analyzer.js @@ -334,12 +334,14 @@ function getViewerData(bundleStats, bundleDir, opts) { // Trying to parse bundle assets and get real module sizes if `bundleDir` is provided /** @type {Record | null} */ let bundlesSources = null; - /** @type {Record | null} */ - let parsedModules = null; + // Parsed sources are stored per asset because different assets can reuse the same module IDs + // (e.g. a root bundle and a worker bundle both numbering their modules from `0`). + /** @type {Record | null} */ + let parsedModulesByAsset = null; if (bundleDir) { bundlesSources = {}; - parsedModules = {}; + parsedModulesByAsset = {}; for (const statAsset of bundleStats.assets) { const assetFile = path.join(bundleDir, statAsset.name); @@ -380,12 +382,12 @@ function getViewerData(bundleStats, bundleDir, opts) { src: bundleInfo.src, runtimeSrc: bundleInfo.runtimeSrc, }; - Object.assign(parsedModules, bundleInfo.modules); + parsedModulesByAsset[statAsset.name] = bundleInfo.modules; } if (Object.keys(bundlesSources).length === 0) { bundlesSources = null; - parsedModules = null; + parsedModulesByAsset = null; logger.warn( "\nNo bundles were parsed. Analyzer will show only original module sizes from stats file.\n", ); @@ -415,6 +417,12 @@ function getViewerData(bundleStats, bundleDir, opts) { ); } + // A module that is a part of more than one asset is one stats object. Every asset that + // contains that module gets the same object. The module tree reads `parsedSrc` late, after + // all assets are processed. Copy the modules for each asset. Then a parsed source that is + // set for one asset cannot change what another asset shows. + assetModules = assetModules.map((statsModule) => ({ ...statsModule })); + const asset = (result[statAsset.name] = /** @type {Asset} */ ({ size: statAsset.size, })); @@ -422,6 +430,11 @@ function getViewerData(bundleStats, bundleDir, opts) { bundlesSources && Object.hasOwn(bundlesSources, statAsset.name) ? bundlesSources[statAsset.name] : null; + const assetParsedModules = + parsedModulesByAsset && + Object.hasOwn(parsedModulesByAsset, statAsset.name) + ? parsedModulesByAsset[statAsset.name] + : null; if (assetSources) { asset.parsedSize = Buffer.byteLength(assetSources.src); @@ -440,16 +453,16 @@ function getViewerData(bundleStats, bundleDir, opts) { } // Adding parsed sources - if (parsedModules) { + if (assetParsedModules) { /** @type {StatsModule[]} */ const unparsedEntryModules = []; for (const statsModule of assetModules) { if ( typeof statsModule.id !== "undefined" && - parsedModules[statsModule.id] + Object.hasOwn(assetParsedModules, statsModule.id) ) { - statsModule.parsedSrc = parsedModules[statsModule.id]; + statsModule.parsedSrc = assetParsedModules[statsModule.id]; } else if (isEntryModule(statsModule)) { unparsedEntryModules.push(statsModule); } diff --git a/test/analyzer.js b/test/analyzer.js index 4980c8f6..f7a5e9de 100644 --- a/test/analyzer.js +++ b/test/analyzer.js @@ -3,7 +3,9 @@ const fs = require("node:fs"); const path = require("node:path"); const url = require("node:url"); const puppeteer = require("puppeteer"); +const Logger = require("../src/Logger"); const { getViewerData } = require("../src/analyzer"); +const { parseBundle } = require("../src/parseUtils"); const { isZstdSupported } = require("../src/sizeUtils"); let browser; @@ -126,6 +128,98 @@ describe("Analyzer", () => { }); }); + it("should not attribute parsed sources across assets that reuse module IDs", () => { + const statsDir = path.resolve( + __dirname, + "./stats/with-worker-loader-dynamic-import", + ); + // `getViewerData` changes the stats it receives, so read a new copy of them. + const stats = JSON.parse( + fs.readFileSync(path.join(statsDir, "stats.json"), "utf8"), + ); + + // Two compilations number their modules from `0`. Module ID `0` is a different module in + // each asset. + const moduleId = "0"; + const rootSource = parseBundle(path.join(statsDir, "bundle.js")).modules[ + moduleId + ]; + const workerSource = parseBundle(path.join(statsDir, "bundle.worker.js")) + .modules[moduleId]; + + expect(rootSource).toEqual(expect.any(String)); + expect(workerSource).not.toBe(rootSource); + + const chartData = getViewerData(stats, statsDir); + const rootAsset = chartData.find((asset) => asset.label === "bundle.js"); + + // `parsedSize` is the length of the parsed source. + expect(rootAsset.groups).toHaveLength(1); + expect(rootAsset.groups[0].parsedSize).toBe(rootSource.length); + }); + + it("should not attribute parsed sources across assets that share a module", () => { + const statsDir = path.resolve( + __dirname, + "./stats/with-module-in-multiple-assets", + ); + // `getViewerData` changes the stats it receives, so read a new copy of them. + const stats = JSON.parse( + fs.readFileSync(path.join(statsDir, "stats.json"), "utf8"), + ); + + // `messages.js` is one source module used by two bundles. Each bundle uses a different + // export, so Webpack creates a different factory for the module in each bundle. + const sharedModuleId = "906"; + const expectedParsedSizes = { + "long-message.js": parseBundle(path.join(statsDir, "long-message.js")) + .modules[sharedModuleId].length, + "short-message.js": parseBundle(path.join(statsDir, "short-message.js")) + .modules[sharedModuleId].length, + }; + + expect(expectedParsedSizes["long-message.js"]).not.toBe( + expectedParsedSizes["short-message.js"], + ); + + const chartData = getViewerData(stats, statsDir); + const parsedSizes = Object.fromEntries( + chartData.map((asset) => [ + asset.label, + asset.groups[0].groups.find((group) => group.label === "messages.js") + .parsedSize, + ]), + ); + + expect(parsedSizes).toEqual(expectedParsedSizes); + }); + + it("should show only stat sizes when no bundle can be parsed", () => { + const statsDir = path.resolve( + __dirname, + "./stats/with-module-in-multiple-assets", + ); + const stats = JSON.parse( + fs.readFileSync(path.join(statsDir, "stats.json"), "utf8"), + ); + const logger = new Logger("silent"); + const warn = jest.spyOn(logger, "warn"); + + // No asset file is in this directory, so every asset fails to parse. + const chartData = getViewerData(stats, path.join(statsDir, "src"), { + logger, + }); + + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("No bundles were parsed"), + ); + + for (const asset of chartData) { + expect(asset.parsedSize).toBeUndefined(); + expect(asset.statSize).toBeGreaterThan(0); + } + }); + it("should update the treemap when a chunk is deselected", async () => { generateReportFrom("with-worker-loader-dynamic-import/stats.json"); const page = await browser.newPage(); diff --git a/test/analyzerUtils.js b/test/analyzerUtils.js index 3e06846d..f3197572 100644 --- a/test/analyzerUtils.js +++ b/test/analyzerUtils.js @@ -2,6 +2,7 @@ const fs = require("node:fs"); const path = require("node:path"); const { getViewerData } = require("../src/analyzer"); +const { getCompressedSize } = require("../src/sizeUtils"); const BUNDLES_DIR = path.resolve(__dirname, "./bundles"); @@ -76,10 +77,22 @@ describe("getViewerData", () => { }, }; - getViewerData(stats, BUNDLES_DIR); + const chartData = getViewerData(stats, BUNDLES_DIR); + const modulesByPath = Object.fromEntries( + chartData[0].groups[0].groups.map((group) => [group.path, group]), + ); - expect(dependencyModule.parsedSrc).toBe(expectedModules[447]); - expect(entryModule.parsedSrc).toBe(expectedModules[956]); + // Asserting on the returned chart data rather than on the input stats objects, because + // `getViewerData` copies the modules of every asset before attributing parsed sources to them. + // Module `parsedSize` is the source length, and the gzip size pins the source itself. + expect(modulesByPath["./src/dependency.js"]).toMatchObject({ + parsedSize: expectedModules[447].length, + gzipSize: getCompressedSize("gzip", expectedModules[447]), + }); + expect(modulesByPath["./src/entry.js"]).toMatchObject({ + parsedSize: expectedModules[956].length, + gzipSize: getCompressedSize("gzip", expectedModules[956]), + }); expect(chunksAccessCount).toBe(1); }); }); diff --git a/test/stats/with-module-in-multiple-assets/README.md b/test/stats/with-module-in-multiple-assets/README.md new file mode 100644 index 00000000..cd027cdf --- /dev/null +++ b/test/stats/with-module-in-multiple-assets/README.md @@ -0,0 +1,45 @@ +# Fixture: one module in two assets + +This fixture tests one source module that Webpack puts in two output bundles. + +## Test case + +The build has two entry files: + +- `src/long-message-entry.js` +- `src/short-message-entry.js` + +Both files import `src/messages.js`. This is the source module that the two bundles share. + +`messages.js` exports a long message and a short message. Each entry file imports only one of +these exports. Webpack creates these bundles: + +- `long-message.js` contains the long message. +- `short-message.js` contains the short message. + +The configuration disables module concatenation, split chunks, and the separate runtime chunk. +These settings keep the module visible in both bundles. + +## Stats and bundle sizes + +Webpack gives `messages.js` module ID `906`. The stats report its source size as `87` in both +chunks. + +Webpack removes the unused export from each bundle. This makes the emitted module factories +different: + +- The factory in `long-message.js` has 59 characters. +- The factory in `short-message.js` has 37 characters. + +The analyzer must report the correct factory size for each bundle. + +## Rebuild + +Run this command from the repository root: + +```sh +npx webpack --config test/stats/with-module-in-multiple-assets/webpack.config.js +``` + +The `stats.json` file is a reduced copy of the Webpack stats. It contains only the fields that the +analyzer reads. diff --git a/test/stats/with-module-in-multiple-assets/long-message.js b/test/stats/with-module-in-multiple-assets/long-message.js new file mode 100644 index 00000000..f8e5d515 --- /dev/null +++ b/test/stats/with-module-in-multiple-assets/long-message.js @@ -0,0 +1 @@ +(()=>{"use strict";var e={906(e,r,o){o.d(r,{U:()=>t});const t="This message is longer."}},r={};function o(t){var s=r[t];if(void 0!==s)return s.exports;var n=r[t]={exports:{}};return e[t](n,n.exports,o),n.exports}o.d=(e,r)=>{for(var t in r)o.o(r,t)&&!o.o(e,t)&&Object.defineProperty(e,t,{enumerable:!0,get:r[t]})},o.o=(e,r)=>Object.prototype.hasOwnProperty.call(e,r);var t=o(906);console.log(t.U)})(); \ No newline at end of file diff --git a/test/stats/with-module-in-multiple-assets/short-message.js b/test/stats/with-module-in-multiple-assets/short-message.js new file mode 100644 index 00000000..fbc10603 --- /dev/null +++ b/test/stats/with-module-in-multiple-assets/short-message.js @@ -0,0 +1 @@ +(()=>{"use strict";var r={906(r,e,o){o.d(e,{S:()=>t});const t="x"}},e={};function o(t){var n=e[t];if(void 0!==n)return n.exports;var s=e[t]={exports:{}};return r[t](s,s.exports,o),s.exports}o.d=(r,e)=>{for(var t in e)o.o(e,t)&&!o.o(r,t)&&Object.defineProperty(r,t,{enumerable:!0,get:e[t]})},o.o=(r,e)=>Object.prototype.hasOwnProperty.call(r,e);var t=o(906);console.log(t.S)})(); \ No newline at end of file diff --git a/test/stats/with-module-in-multiple-assets/src/long-message-entry.js b/test/stats/with-module-in-multiple-assets/src/long-message-entry.js new file mode 100644 index 00000000..12a592c7 --- /dev/null +++ b/test/stats/with-module-in-multiple-assets/src/long-message-entry.js @@ -0,0 +1,3 @@ +import { longMessage } from "./messages.js"; + +console.log(longMessage); diff --git a/test/stats/with-module-in-multiple-assets/src/messages.js b/test/stats/with-module-in-multiple-assets/src/messages.js new file mode 100644 index 00000000..5621c2a7 --- /dev/null +++ b/test/stats/with-module-in-multiple-assets/src/messages.js @@ -0,0 +1,2 @@ +export const longMessage = "This message is longer."; +export const shortMessage = "x"; diff --git a/test/stats/with-module-in-multiple-assets/src/short-message-entry.js b/test/stats/with-module-in-multiple-assets/src/short-message-entry.js new file mode 100644 index 00000000..df7ce38d --- /dev/null +++ b/test/stats/with-module-in-multiple-assets/src/short-message-entry.js @@ -0,0 +1,3 @@ +import { shortMessage } from "./messages.js"; + +console.log(shortMessage); diff --git a/test/stats/with-module-in-multiple-assets/stats.json b/test/stats/with-module-in-multiple-assets/stats.json new file mode 100644 index 00000000..e597ccff --- /dev/null +++ b/test/stats/with-module-in-multiple-assets/stats.json @@ -0,0 +1,80 @@ +{ + "assets": [ + { + "type": "asset", + "name": "long-message.js", + "size": 400, + "chunks": [379], + "info": { "javascriptModule": false } + }, + { + "type": "asset", + "name": "short-message.js", + "size": 378, + "chunks": [453], + "info": { "javascriptModule": false } + } + ], + "chunks": [ + { + "id": 379, + "names": ["long-message"], + "files": ["long-message.js"], + "modules": [ + { + "id": 907, + "identifier": "./src/long-message-entry.js", + "name": "./src/long-message-entry.js", + "size": 72, + "depth": 0, + "chunks": [379], + "usedExports": [] + }, + { + "id": 906, + "identifier": "./src/messages.js", + "name": "./src/messages.js", + "size": 87, + "depth": 1, + "chunks": [379, 453], + "usedExports": ["longMessage"] + } + ] + }, + { + "id": 453, + "names": ["short-message"], + "files": ["short-message.js"], + "modules": [ + { + "id": 906, + "identifier": "./src/messages.js", + "name": "./src/messages.js", + "size": 87, + "depth": 1, + "chunks": [379, 453], + "usedExports": ["shortMessage"] + }, + { + "id": 645, + "identifier": "./src/short-message-entry.js", + "name": "./src/short-message-entry.js", + "size": 74, + "depth": 0, + "chunks": [453], + "usedExports": [] + } + ] + } + ], + "entrypoints": { + "long-message": { + "name": "long-message", + "assets": [{ "name": "long-message.js" }] + }, + "short-message": { + "name": "short-message", + "assets": [{ "name": "short-message.js" }] + } + } +} diff --git a/test/stats/with-module-in-multiple-assets/webpack.config.js b/test/stats/with-module-in-multiple-assets/webpack.config.js new file mode 100644 index 00000000..9541d2e3 --- /dev/null +++ b/test/stats/with-module-in-multiple-assets/webpack.config.js @@ -0,0 +1,21 @@ +const path = require("node:path"); + +// Build two bundles that import different exports from the same module. +module.exports = { + mode: "production", + context: __dirname, + entry: { + "long-message": "./src/long-message-entry.js", + "short-message": "./src/short-message-entry.js", + }, + output: { + path: __dirname, + filename: "[name].js", + }, + optimization: { + concatenateModules: false, + runtimeChunk: false, + splitChunks: false, + moduleIds: "deterministic", + }, +};