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..79489a99 100644 --- a/packages/bundlecheck/src/bundler.ts +++ b/packages/bundlecheck/src/bundler.ts @@ -273,25 +273,40 @@ 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 +641,15 @@ 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..8dd8b10a 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. + /** + * 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: {