From 0b52cf73b45ebd614157fa150caf24f58290e2f2 Mon Sep 17 00:00:00 2001 From: Arno V Date: Sun, 1 Feb 2026 09:52:42 -0500 Subject: [PATCH 1/2] fix(bundlechecker): Only externalize React/react-dom when declared Make React/react-dom externalization conditional on the package's declared dependencies/peerDependencies. Updates bundler.getExternals to accept packageDependencies and only mark react/react-dom as externals when present (and never externalize the package being checked). Pass package dependency info into getExternals from checkBundleSize and adjust the cache key logic to reflect user-specified externals rather than computed defaults. Update tests to cover dependency/peerDependency cases and react/react-dom self-checks, and refresh README/CLI help text to document the new "smart externalization" behavior. --- packages/bundlecheck/README.md | 14 ++--- .../bundlecheck/src/__tests__/bundler.test.ts | 51 ++++++++++++++++--- packages/bundlecheck/src/bundler.ts | 29 +++++++---- packages/bundlecheck/src/index.ts | 15 +++--- packages/bundlecheck/src/parse.ts | 9 ++-- 5 files changed, 83 insertions(+), 35 deletions(-) diff --git a/packages/bundlecheck/README.md b/packages/bundlecheck/README.md index 987ba2eb..b57fa8ff 100644 --- a/packages/bundlecheck/README.md +++ b/packages/bundlecheck/README.md @@ -11,7 +11,7 @@ A CLI tool to check the bundle size of npm packages, similar to [bundlephobia.co - Interactive version selection with `--versions` flag - Bundle size trend analysis with `--trend` flag (bar graph across versions) - Support for checking specific exports (tree-shaking) -- Automatic externalization of React and React-DOM +- Smart externalization of React and React-DOM (only when declared as dependencies) - Raw and gzip sizes with configurable compression level - **Platform support**: target `browser` (default) or `node` with smart auto-detection - Custom npm registry support (for private registries) @@ -122,7 +122,7 @@ bundlecheck @versini/ui-panel@1.0.0 # Check specific exports from @mantine/core bundlecheck @mantine/core "ScrollArea,Button" -# Check react itself (without marking it as external) +# Check react with all dependencies bundled (no externals) bundlecheck react -n # Add vue and svelte as additional externals @@ -181,7 +181,7 @@ console.log(stats); // rawSizeFormatted: "229.07 kB", // gzipSizeFormatted: "44.61 kB", // exports: [], -// externals: ["react", "react-dom"], +// externals: ["react", "react-dom"], // Only present because @mantine/core has these as peerDependencies // dependencies: ["@floating-ui/react", ...], // platform: "browser", // gzipLevel: 5, @@ -241,7 +241,7 @@ Get bundle size statistics for a single package. | `package` | `string` | (required) | Package name with optional version (e.g., `lodash@4.17.0`) | | `exports` | `string[]` | `undefined`| Specific exports to measure (tree-shaking) | | `external` | `string[]` | `undefined`| Additional packages to mark as external | -| `noExternal` | `boolean` | `false` | Bundle everything including default externals | +| `noExternal` | `boolean` | `false` | Bundle everything (no externals, even react/react-dom) | | `gzipLevel` | `number` | `5` | Gzip compression level (1-9) | | `registry` | `string` | `undefined`| Custom npm registry URL | | `platform` | `"browser" \| "node" \| "auto"` | `"auto"` | Target platform | @@ -386,9 +386,11 @@ bundlecheck fastify -p server # "server" is an alias for "node" ## Default Externals -By default, `react` and `react-dom` are marked as external (not included in the bundle size) since most React-based packages expect these as peer dependencies. This matches how these packages would typically be used in a real application. +When a package declares `react` or `react-dom` in its `dependencies` or `peerDependencies`, they are automatically marked as external (not included in the bundle size). This matches how these packages would typically be used in a real application where React is provided by the host application. -To include React/React-DOM in the bundle size calculation, use the `--no-external` flag. +For packages that don't depend on React, these are not automatically externalized. + +To include all dependencies (including React when present) in the bundle size calculation, use the `--no-external` flag. ## Custom Registry diff --git a/packages/bundlecheck/src/__tests__/bundler.test.ts b/packages/bundlecheck/src/__tests__/bundler.test.ts index f1b5de78..b15dc29d 100644 --- a/packages/bundlecheck/src/__tests__/bundler.test.ts +++ b/packages/bundlecheck/src/__tests__/bundler.test.ts @@ -199,8 +199,9 @@ describe("checkBundleSize", () => { }); expect(result.rawSize).toBeGreaterThan(0); expect(result.gzipSize).toBeGreaterThan(0); - expect(result.externals).toContain("react"); - expect(result.externals).toContain("react-dom"); + // react and react-dom are NOT external since the package doesn't have them as dependencies + expect(result.externals).not.toContain("react"); + expect(result.externals).not.toContain("react-dom"); }); it("should respect noExternal option", async () => { @@ -218,27 +219,61 @@ describe("checkBundleSize", () => { additionalExternals: ["lodash", "dayjs"], }); - expect(result.externals).toContain("react"); - expect(result.externals).toContain("react-dom"); + // react and react-dom are NOT external since the package doesn't have them as dependencies + expect(result.externals).not.toContain("react"); + expect(result.externals).not.toContain("react-dom"); + // Additional externals are added expect(result.externals).toContain("lodash"); expect(result.externals).toContain("dayjs"); }); - it("should exclude react from externals when checking react itself", async () => { + it("should not mark react as external when checking react itself", async () => { const result = await checkBundleSize({ packageName: "react", }); + // When checking react, it should never be external (even if it were somehow in deps) expect(result.externals).not.toContain("react"); + }); + + it("should include react/react-dom as external when package has them as peer dependencies", async () => { + vi.mocked(fs.readFileSync).mockReturnValue( + JSON.stringify({ + name: "test-package", + version: "1.0.0", + main: "./dist/index.js", + dependencies: {}, + peerDependencies: { react: "^18.0.0", "react-dom": "^18.0.0" }, + }), + ); + + const result = await checkBundleSize({ + packageName: "test-package", + }); + + // react and react-dom ARE external since they're in peerDependencies + expect(result.externals).toContain("react"); expect(result.externals).toContain("react-dom"); }); - it("should exclude react-dom from externals when checking react-dom itself", async () => { + it("should include react as external when package has it as prod dependency", async () => { + vi.mocked(fs.readFileSync).mockReturnValue( + JSON.stringify({ + name: "test-package", + version: "1.0.0", + main: "./dist/index.js", + dependencies: { react: "^18.0.0" }, + peerDependencies: {}, + }), + ); + const result = await checkBundleSize({ - packageName: "react-dom", + packageName: "test-package", }); + // react IS external since it's in dependencies expect(result.externals).toContain("react"); + // react-dom is NOT external since it's not in dependencies expect(result.externals).not.toContain("react-dom"); }); @@ -275,6 +310,8 @@ describe("checkBundleSize", () => { }); expect(result.dependencies).toContain("react"); + // react should be external since it's in peerDependencies + expect(result.externals).toContain("react"); }); it("should respect custom gzip level", async () => { diff --git a/packages/bundlecheck/src/bundler.ts b/packages/bundlecheck/src/bundler.ts index 2ca2e2b6..1f42ecc3 100644 --- a/packages/bundlecheck/src/bundler.ts +++ b/packages/bundlecheck/src/bundler.ts @@ -273,25 +273,34 @@ function generateEntryContent(options: EntryContentOptions): string { } /** - * Get externals list based on options. + * Get externals list based on options and package dependencies. + * react and react-dom are only marked as external if they are declared + * in the package's dependencies or peerDependencies. */ export function getExternals( packageName: string, externals?: string[], noExternal?: boolean, + packageDependencies?: string[], ): string[] { if (noExternal) { return []; } - // Start with default externals (react, react-dom). - let result = [...DEFAULT_EXTERNALS]; + // Start with empty result - we'll only add react/react-dom if they're in package deps. + let result: string[] = []; - // If checking react or react-dom themselves, don't mark them as external. - if (packageName === "react") { - result = result.filter((e) => e !== "react"); - } else if (packageName === "react-dom") { - result = result.filter((e) => e !== "react-dom"); + // Only include react/react-dom if they're in the package's dependencies or peerDependencies. + if (packageDependencies && packageDependencies.length > 0) { + for (const dep of DEFAULT_EXTERNALS) { + // Don't mark as external if we're checking the package itself. + if (dep === packageName) { + continue; + } + if (packageDependencies.includes(dep)) { + result.push(dep); + } + } } // Add any additional externals. @@ -626,11 +635,13 @@ export async function checkBundleSize( const entryFile = path.join(tmpDir, "entry.js"); fs.writeFileSync(entryFile, entryContent); - // Get externals. + // Get externals based on package dependencies. + // Only include react/react-dom if they're in the package's dependencies or peerDependencies. const externals = getExternals( packageName, additionalExternals, noExternal, + allDependencies, ); // Bundle with esbuild. diff --git a/packages/bundlecheck/src/index.ts b/packages/bundlecheck/src/index.ts index e8a92e25..844ada24 100644 --- a/packages/bundlecheck/src/index.ts +++ b/packages/bundlecheck/src/index.ts @@ -9,7 +9,6 @@ import { type BundleResult, checkBundleSize, formatBytes, - getExternals, parsePackageSpecifier, } from "./bundler.js"; import { @@ -45,7 +44,7 @@ export type GetBundleStatsOptions = { */ external?: string[]; /** - * Bundle everything including default externals (react, react-dom). + * Bundle everything including dependencies that would normally be external. */ noExternal?: boolean; /** @@ -142,7 +141,7 @@ export type GetBundleTrendOptions = { */ external?: string[]; /** - * Bundle everything including default externals (react, react-dom). + * Bundle everything including dependencies that would normally be external. */ noExternal?: boolean; /** @@ -329,17 +328,19 @@ export async function getBundleStats( resolvedVersion = tags.latest || requestedVersion; } - // Compute externals for cache key. - const externals = getExternals(baseName, additionalExternals, noExternal); - // Build cache key. + // Note: We use additionalExternals here, not computed externals, because + // the default externals (react, react-dom) are determined at bundle time + // based on the package's dependencies. The cache key captures the user's intent + // (additionalExternals + noExternal flag), and the actual externals are stored + // in the cached result. const cacheKey = normalizeCacheKey({ packageName: baseName, version: resolvedVersion, exports: exportsList, platform, gzipLevel, - externals, + externals: additionalExternals || [], noExternal: noExternal ?? false, }); diff --git a/packages/bundlecheck/src/parse.ts b/packages/bundlecheck/src/parse.ts index 9b180bfd..01ee2eca 100644 --- a/packages/bundlecheck/src/parse.ts +++ b/packages/bundlecheck/src/parse.ts @@ -1,10 +1,6 @@ /* istanbul ignore file */ import { parser } from "@node-cli/parser"; -import { - DEFAULT_EXTERNALS, - defaultFlags, - isValidPlatform, -} from "./defaults.js"; +import { defaultFlags, isValidPlatform } from "./defaults.js"; export type Flags = { boring?: boolean; @@ -101,7 +97,8 @@ export const config: Configuration = parser({ external: { shortFlag: "e", default: defaultFlags.external, - description: `Comma-separated additional externals (default externals: ${DEFAULT_EXTERNALS.join(", ")})`, + description: + "Comma-separated additional externals (react, react-dom auto-external when in package deps)", type: "string", }, noExternal: { From 7d38adf5d8b5b9e606589620c6fa1e80aeae3da2 Mon Sep 17 00:00:00 2001 From: Arno V Date: Sun, 1 Feb 2026 09:56:10 -0500 Subject: [PATCH 2/2] Refactor comments to JSDoc-style blocks Reformatted several inline // comments into multiline /** ... */ block/JSDoc-style comments for clarity and line-wrapping. Changes touch getExternals and checkBundleSize in packages/bundlecheck/src/bundler.ts and the cache key explanation in packages/bundlecheck/src/index.ts. No functional behavior was changed. --- packages/bundlecheck/src/bundler.ts | 22 +++++++++++++++------- packages/bundlecheck/src/index.ts | 12 ++++++------ 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/bundlecheck/src/bundler.ts b/packages/bundlecheck/src/bundler.ts index 1f42ecc3..79489a99 100644 --- a/packages/bundlecheck/src/bundler.ts +++ b/packages/bundlecheck/src/bundler.ts @@ -273,9 +273,9 @@ function generateEntryContent(options: EntryContentOptions): string { } /** - * Get externals list based on options and package dependencies. - * react and react-dom are only marked as external if they are declared - * in the package's dependencies or peerDependencies. + * Get externals list based on options and package dependencies. react and + * react-dom are only marked as external if they are declared in the package's + * dependencies or peerDependencies. */ export function getExternals( packageName: string, @@ -287,10 +287,16 @@ export function getExternals( return []; } - // Start with empty result - we'll only add react/react-dom if they're in package deps. + /** + * Start with empty result - we'll only add react/react-dom if they're in + * package deps. + */ let result: string[] = []; - // Only include react/react-dom if they're in the package's dependencies or peerDependencies. + /** + * Only include react/react-dom if they're in the package's dependencies or + * peerDependencies. + */ if (packageDependencies && packageDependencies.length > 0) { for (const dep of DEFAULT_EXTERNALS) { // Don't mark as external if we're checking the package itself. @@ -635,8 +641,10 @@ export async function checkBundleSize( const entryFile = path.join(tmpDir, "entry.js"); fs.writeFileSync(entryFile, entryContent); - // Get externals based on package dependencies. - // Only include react/react-dom if they're in the package's dependencies or peerDependencies. + /** + * Get externals based on package dependencies. Only include react/react-dom + * if they're in the package's dependencies or peerDependencies. + */ const externals = getExternals( packageName, additionalExternals, diff --git a/packages/bundlecheck/src/index.ts b/packages/bundlecheck/src/index.ts index 844ada24..8dd8b10a 100644 --- a/packages/bundlecheck/src/index.ts +++ b/packages/bundlecheck/src/index.ts @@ -328,12 +328,12 @@ export async function getBundleStats( resolvedVersion = tags.latest || requestedVersion; } - // Build cache key. - // Note: We use additionalExternals here, not computed externals, because - // the default externals (react, react-dom) are determined at bundle time - // based on the package's dependencies. The cache key captures the user's intent - // (additionalExternals + noExternal flag), and the actual externals are stored - // in the cached result. + /** + * Build cache key. + * NOTE: We use additionalExternals here, not computed externals, because the default externals (react, react-dom) are determined at bundle time based on the package's dependencies. + * The cache key captures the user's intent (additionalExternals + noExternal + * flag), and the actual externals are stored in the cached result. + */ const cacheKey = normalizeCacheKey({ packageName: baseName, version: resolvedVersion,