diff --git a/HISTORY.md b/HISTORY.md index 83b5a3ab..4b9923f2 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,11 @@ History ======= +## UNRELEASED + +- BUG: A loader in the `identifier` field would incorrectly have all modules inferred "as a `node_modules` file", even if not. Implements a naive loader stripping heuristic to correctly assess if `node_modules` or real application source. +- Optimizes internal calls to `_isNodeModules()` from 2 to 1 for better performance. + ## 4.1.0 - Add `emitHandler` option to `DuplicatesPlugin` to allow customized output. diff --git a/README.md b/README.md index ccfd95ca..0854af55 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ It is also the engine for the handy [`webpack-dashboard`](https://github.com/For - [`duplicates`](#duplicates) - [`versions`](#versions) - [`sizes`](#sizes) +- [Notes, tips, tricks](#notes-tips-tricks) - [Other useful tools](#other-useful-tools) ## Plugin @@ -467,6 +468,14 @@ inspectpack --action=sizes The sizes reported are most likely of the uncompressed source of each module. Because `inspectpack` relies on the `stats` object output, the information reported in the sizes action reflects at what point the `stats` object was generated. For example, using the recommended `webpack-stats-plugin`, the source information would be after all loader processing, but potentially before any webpack plugins. Thus, the resultant, _actual_ size of a given module in your ultimate bundle could be bigger (e.g., in a development bundle with webpack-inserted comments and imports) or smaller (e.g., your bundle is minified and gzipped). +## Notes, tips, tricks + +### Special characters in file paths + +Webpack loaders use a special syntax for loaders with `?` and `!` characters that will end up in the stats object `identifier` field (e.g., `/PATH/TO/node_modules/css-loader/index.js??ref--7-1!/PATH/TO/node_modules/postcss-loader/lib/index.js??ref--7-2!/PATH/TO/src/bar/my-style.css"`) for a given module item. + +We currently use a very naive solution to determine the "true" asset name by just stripping off everything before the last `?`/`!` character. There are technically some potential use cases (e.g. those characters in real file paths) that might not be correctly handled. We have a [tracking ticket](https://github.com/FormidableLabs/inspectpack/issues/98) for folks to comment on if you're hitting any issues. + ## Other useful tools Other tools that inspect Webpack bundles: diff --git a/src/lib/actions/base.ts b/src/lib/actions/base.ts index 5d11a960..06245a5e 100644 --- a/src/lib/actions/base.ts +++ b/src/lib/actions/base.ts @@ -45,18 +45,30 @@ export const nodeModulesParts = (name: string) => toPosixPath(name).split(NM_RE) // True if name is part of a `node_modules` path. export const _isNodeModules = (name: string): boolean => nodeModulesParts(name).length > 1; +// Naively remove any prefix portion of an identifier, namely: +// - `REMOVE?KEEP` +// - `REMOVE!KEEP` +export const _normalizeIdentifier = (name: string): string => { + const lastBang = name.lastIndexOf("!"); + const lastQuestion = name.lastIndexOf("?"); + const prefixEnd = Math.max(lastBang, lastQuestion); + + // Remove prefix here. + if (prefixEnd > -1) { + return name.substr(prefixEnd + 1); + } + + return name; +}; + // Convert a `node_modules` name to a base name. // +// **Note**: Assumes only passed `node_modules` values. +// // Normalizations: // - Remove starting path if `./` // - Switch Windows paths to Mac/Unix style. -// - Non-`node_modules` sources (e.g. "your" sources) return `null`. export const _getBaseName = (name: string): string | null => { - // Not in `node_modules`. - if (!_isNodeModules(name)) { - return null; - } - // Slice to just after last occurrence of node_modules. const parts = nodeModulesParts(name); const lastName = parts[parts.length - 1]; @@ -133,42 +145,51 @@ export abstract class Action { // Add in any parent chunks and ensure unique array. const chunks = Array.from(new Set(mod.chunks.concat(parentChunks || []))); - if (RWebpackStatsModuleSource.decode(mod).isRight()) { - // Easy case -- a normal source code module. - const srcMod = mod as IWebpackStatsModuleSource; - const { identifier, size, source } = srcMod; - - return list.concat([{ - baseName: _getBaseName(identifier), - chunks, - identifier, - isNodeModules: _isNodeModules(identifier), - isSynthetic: false, - size, - source, - }]); - } else if (RWebpackStatsModuleModules.decode(mod).isRight()) { + // Fields + let isSynthetic = false; + let source = null; + let identifier; + let size; + + if (RWebpackStatsModuleModules.decode(mod).isRight()) { // Recursive case -- more modules. const modsMod = mod as IWebpackStatsModuleModules; + // Return and recurse. return list.concat(this.getSourceMods(modsMod.modules, chunks)); + + } else if (RWebpackStatsModuleSource.decode(mod).isRight()) { + // Easy case -- a normal source code module. + const srcMod = mod as IWebpackStatsModuleSource; + identifier = srcMod.identifier; + size = srcMod.size; + source = srcMod.source; + } else if (RWebpackStatsModuleSynthetic.decode(mod).isRight()) { // Catch-all case -- a module without modules or source. const syntheticMod = mod as IWebpackStatsModuleSynthetic; - const { identifier, size } = syntheticMod; - - return list.concat([{ - baseName: _getBaseName(identifier), - chunks, - identifier, - isNodeModules: _isNodeModules(identifier), - isSynthetic: true, - size, - source: null, - }]); + identifier = syntheticMod.identifier; + size = syntheticMod.size; + isSynthetic = true; + + } else { + throw new Error(`Cannot match to known module type: ${JSON.stringify(mod)}`); } - throw new Error(`Cannot match to known module type: ${JSON.stringify(mod)}`); + // We've now got a single entry to prepare and add. + const normalizedId = _normalizeIdentifier(identifier as string); + const isNodeModules = _isNodeModules(normalizedId); + const baseName = isNodeModules ? _getBaseName(normalizedId) : null; + + return list.concat([{ + baseName, + chunks, + identifier, + isNodeModules, + isSynthetic, + size, + source, + }]); }, [], ) diff --git a/test/lib/actions/base.spec.ts b/test/lib/actions/base.spec.ts index ad2de5c6..6fe570fa 100644 --- a/test/lib/actions/base.spec.ts +++ b/test/lib/actions/base.spec.ts @@ -1,48 +1,6 @@ -import { _getBaseName, _isNodeModules } from "../../../src/lib/actions/base"; +import { _getBaseName, _isNodeModules, _normalizeIdentifier } from "../../../src/lib/actions/base"; describe("lib/actions/base", () => { - describe("_getBaseName", () => { - it("handles non-node_modules files", () => { - expect(_getBaseName("")).to.equal(null); - expect(_getBaseName("foo.js")).to.equal(null); - expect(_getBaseName("./foo.js")).to.equal(null); - expect(_getBaseName(".\\foo.js")).to.equal(null); - expect(_getBaseName("bar/foo.js")).to.equal(null); - expect(_getBaseName("bar\\foo.js")).to.equal(null); - expect(_getBaseName("node_modulesM/bar/foo.js")).to.equal(null); - expect(_getBaseName("node_modulesM\\bar\\foo.js")).to.equal(null); - }); - - it("removes node_modules", () => { - expect(_getBaseName("./node_modules/foo.js")).to.equal("foo.js"); - expect(_getBaseName(".\\node_modules\\foo.js")).to.equal("foo.js"); - expect(_getBaseName("node_modules/bar/foo.js")).to.equal("bar/foo.js"); - expect(_getBaseName("node_modules\\bar\\foo.js")).to.equal("bar/foo.js"); - }); - - it("removes repeated node_modules", () => { - expect(_getBaseName("./node_modules/baz/node_modules/foo.js")).to.equal("foo.js"); - expect(_getBaseName(".\\node_modules\\baz\\node_modules\\foo.js")).to.equal("foo.js"); - expect(_getBaseName("bruh/node_modules/bar/foo.js")).to.equal("bar/foo.js"); - expect(_getBaseName("bruh\\node_modules\\bar\\foo.js")).to.equal("bar/foo.js"); - }); - - it("handles synthetic modules", () => { - expect(_getBaseName("node_modules/moment/locale sync /es/")) - .to.equal("moment/locale sync /es/"); - expect(_getBaseName("node_modules\\moment/locale sync /es/")) - .to.equal("moment/locale sync /es/"); - }); - - // All of this behavior is negotiable. - it("handles weird cases that should never come up", () => { - expect(_getBaseName("node_modules")).to.equal(""); - expect(_getBaseName("node_modules/")).to.equal(""); - expect(_getBaseName("./node_modules")).to.equal(""); - expect(_getBaseName("./foo/bar/node_modules")).to.equal(""); - }); - }); - describe("_isNodeModules", () => { it("handles base cases", () => { expect(_isNodeModules("")).to.equal(false); @@ -77,4 +35,64 @@ describe("lib/actions/base", () => { expect(_isNodeModules("./foo/bar/node_modules")).to.equal(true); }); }); + + describe("_normalizeIdentifier", () => { + it("handles base cases", () => { + expect(_normalizeIdentifier("")).to.equal(""); + expect(_normalizeIdentifier("foo.js")).to.equal("foo.js"); + expect(_normalizeIdentifier("/foo.js")).to.equal("/foo.js"); + expect(_normalizeIdentifier("\\foo.js")).to.equal("\\foo.js"); + expect(_normalizeIdentifier("bar/foo.js")).to.equal("bar/foo.js"); + expect(_normalizeIdentifier("bar\\foo.js")).to.equal("bar\\foo.js"); + expect(_normalizeIdentifier("/bar/foo.js")).to.equal("/bar/foo.js"); + expect(_normalizeIdentifier("x:\\bar\\foo.js")).to.equal("x:\\bar\\foo.js"); + }); + + // tslint:disable max-line-length + it("handles loaders", () => { + expect(_normalizeIdentifier("/PATH/TO/node_modules/css-loader/lib/css-base.js")) + .to.equal("/PATH/TO/node_modules/css-loader/lib/css-base.js"); + expect(_normalizeIdentifier("/PATH/TO/node_modules/next/dist/build/webpack/loaders/next-babel-loader.js??ref--4!/PATH/TO/node_modules/pkg/foo.js")) + .to.equal("/PATH/TO/node_modules/pkg/foo.js"); + expect(_normalizeIdentifier("/PATH/TO/node_modules/css-loader/index.js??ref--7-1!/PATH/TO/node_modules/postcss-loader/lib/index.js??ref--7-2!/PATH/TO/node_modules/@scope/foo/package.css")) + .to.equal("/PATH/TO/node_modules/@scope/foo/package.css"); + + expect(_normalizeIdentifier("/PATH/TO/node_modules/next/dist/build/webpack/loaders/next-babel-loader.js??ref--4!/PATH/TO/src/modules/debug/foo.js")) + .to.equal("/PATH/TO/src/modules/debug/foo.js"); + expect(_normalizeIdentifier("/PATH/TO/node_modules/css-loader/index.js??ref--7-1!/PATH/TO/node_modules/postcss-loader/lib/index.js??ref--7-2!/PATH/TO/src/bar/my-style.css")) + .to.equal("/PATH/TO/src/bar/my-style.css"); + }); + // tslint:enable max-line-length + }); + + describe("_getBaseName", () => { + it("removes node_modules", () => { + expect(_getBaseName("./node_modules/foo.js")).to.equal("foo.js"); + expect(_getBaseName(".\\node_modules\\foo.js")).to.equal("foo.js"); + expect(_getBaseName("node_modules/bar/foo.js")).to.equal("bar/foo.js"); + expect(_getBaseName("node_modules\\bar\\foo.js")).to.equal("bar/foo.js"); + }); + + it("removes repeated node_modules", () => { + expect(_getBaseName("./node_modules/baz/node_modules/foo.js")).to.equal("foo.js"); + expect(_getBaseName(".\\node_modules\\baz\\node_modules\\foo.js")).to.equal("foo.js"); + expect(_getBaseName("bruh/node_modules/bar/foo.js")).to.equal("bar/foo.js"); + expect(_getBaseName("bruh\\node_modules\\bar\\foo.js")).to.equal("bar/foo.js"); + }); + + it("handles synthetic modules", () => { + expect(_getBaseName("node_modules/moment/locale sync /es/")) + .to.equal("moment/locale sync /es/"); + expect(_getBaseName("node_modules\\moment/locale sync /es/")) + .to.equal("moment/locale sync /es/"); + }); + + // All of this behavior is negotiable. + it("handles weird cases that should never come up", () => { + expect(_getBaseName("node_modules")).to.equal(""); + expect(_getBaseName("node_modules/")).to.equal(""); + expect(_getBaseName("./node_modules")).to.equal(""); + expect(_getBaseName("./foo/bar/node_modules")).to.equal(""); + }); + }); });