Warn for scripts larger than 1MB compressed (#2130)

* Warn for scripts larger than 1MB compressed Prints a warning for scripts > 1MB compressed, encouraging smaller script sizes. If a publish fails with either a script size error or a validator error on script startup (CPU or memory), we print out the largest 5 dependencies in your bundle. This is accomplished by using the esbuild generated metafile. * Add NO_SCRIPT_SIZE_WARNING env variable Will skip the warning about bundle sizes when your bundle exceeds 1MB gzipped if set * Add changeset Co-authored-by: Matthew Rodgers <matthewrodgers@cloudflare.com>
cloudflare · Nov 8, 2022 · 68f4fa6 · 68f4fa6
1 parent 1987a79
commit 68f4fa6
Show file tree

Hide file tree

Showing 7 changed files with 335 additions and 33 deletions.
diff --git a/.changeset/metal-news-tap.md b/.changeset/metal-news-tap.md
@@ -0,0 +1,14 @@
+---
+"wrangler": minor
+---
+
+feature: Add warnings around bundle sizes for large scripts
+
+Prints a warning for scripts > 1MB compressed, encouraging smaller
+script sizes. This warning can be silenced by setting the
+NO_SCRIPT_SIZE_WARNING env variable
+
+If a publish fails with either a script size error or a validator error
+on script startup (CPU or memory), we print out the largest 5
+dependencies in your bundle. This is accomplished by using the esbuild
+generated metafile.
diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts
@@ -1,7 +1,13 @@
+import { Buffer } from "node:buffer";
+import { randomFillSync } from "node:crypto";
 import * as fs from "node:fs";
 import * as path from "node:path";
 import * as TOML from "@iarna/toml";
 import * as esbuild from "esbuild";
+import {
+	printBundleSize,
+	printOffendingDependencies,
+} from "../bundle-reporter";
 import { writeAuthConfigFile } from "../user";
 import { mockAccountId, mockApiToken } from "./helpers/mock-account-id";
 import {
@@ -6418,6 +6424,200 @@ addEventListener('fetch', event => {});`
 			        }
 		      `);
 		});
+
+		test("should check biggest dependencies when upload fails with script size error", async () => {
+			fs.writeFileSync("dependency.js", `export const thing = "a string dep";`);
+
+			fs.writeFileSync(
+				"index.js",
+				`import { thing } from "./dependency";
+
+        export default {
+          async fetch() {
+            return new Response('response plus ' + thing);
+          }
+        }`
+			);
+			setMockRawResponse(
+				"/accounts/:accountId/workers/scripts/:scriptName",
+				"PUT",
+				() => {
+					return createFetchResult({}, false, [
+						{ code: 10027, message: "workers.api.error.script_too_large" },
+					]);
+				}
+			);
+			writeWranglerToml({
+				main: "index.js",
+			});
+			mockSubDomainRequest();
+			mockUploadWorkerRequest();
+			await expect(runWrangler("publish")).rejects.toMatchInlineSnapshot(
+				`[ParseError: A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name) failed.]`
+			);
+			expect(std).toMatchInlineSnapshot(`
+			Object {
+			  "debug": "",
+			  "err": "",
+			  "out": "Total Upload: xx KiB / gzip: xx KiB
+
+			[31mX [41;31m[[41;97mERROR[41;31m][0m [1mA request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name) failed.[0m
+
+			  workers.api.error.script_too_large [code: 10027]
+
+			  If you think this is a bug, please open an issue at:
+			  [4mhttps://github.com/cloudflare/wrangler2/issues/new/choose[0m
+
+			",
+			  "warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mHere are the 2 largest dependencies included in your script:[0m
+
+			  - index.js - xx KiB
+			  - dependency.js - xx KiB
+			  If these are unnecessary, consider removing them
+
+			",
+			}
+		`);
+		});
+
+		test("should check biggest dependencies when upload fails with script startup error", async () => {
+			fs.writeFileSync("dependency.js", `export const thing = "a string dep";`);
+
+			fs.writeFileSync(
+				"index.js",
+				`import { thing } from "./dependency";
+
+        export default {
+          async fetch() {
+            return new Response('response plus ' + thing);
+          }
+        }`
+			);
+			setMockRawResponse(
+				"/accounts/:accountId/workers/scripts/:scriptName",
+				"PUT",
+				() => {
+					return createFetchResult({}, false, [
+						{
+							code: 10021,
+							message: "Error: Script startup exceeded CPU time limit.",
+						},
+					]);
+				}
+			);
+			writeWranglerToml({
+				main: "index.js",
+			});
+			mockSubDomainRequest();
+			mockUploadWorkerRequest();
+			await expect(runWrangler("publish")).rejects.toMatchInlineSnapshot(
+				`[ParseError: A request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name) failed.]`
+			);
+			expect(std).toMatchInlineSnapshot(`
+			Object {
+			  "debug": "",
+			  "err": "",
+			  "out": "Total Upload: xx KiB / gzip: xx KiB
+
+			[31mX [41;31m[[41;97mERROR[41;31m][0m [1mA request to the Cloudflare API (/accounts/some-account-id/workers/scripts/test-name) failed.[0m
+
+			  Error: Script startup exceeded CPU time limit. [code: 10021]
+
+			  If you think this is a bug, please open an issue at:
+			  [4mhttps://github.com/cloudflare/wrangler2/issues/new/choose[0m
+
+			",
+			  "warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mHere are the 2 largest dependencies included in your script:[0m
+
+			  - index.js - xx KiB
+			  - dependency.js - xx KiB
+			  If these are unnecessary, consider removing them
+
+			",
+			}
+		`);
+		});
+
+		describe("unit tests", () => {
+			// keeping these as unit tests to try and keep them snappy, as they often deal with
+			// big files that would take a while to deal with in a full wrangler test
+
+			test("should print the bundle size and warn about large scripts when > 1MiB", async () => {
+				const bigModule = Buffer.alloc(10_000_000);
+				randomFillSync(bigModule);
+				await printBundleSize({ name: "index.js", content: "" }, [
+					{ name: "index.js", content: bigModule, type: "buffer" },
+				]);
+
+				expect(std).toMatchInlineSnapshot(`
+			Object {
+			  "debug": "",
+			  "err": "",
+			  "out": "Total Upload: xx KiB / gzip: xx KiB",
+			  "warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mWe recommend keeping your script less than 1MiB (1024 KiB) after gzip. Exceeding past this can affect cold start time[0m
+
+			",
+			}
+		`);
+			});
+
+			test("should not warn about bundle sizes when NO_SCRIPT_SIZE_WARNING is set", async () => {
+				const previousValue = process.env.NO_SCRIPT_SIZE_WARNING;
+				process.env.NO_SCRIPT_SIZE_WARNING = "true";
+
+				const bigModule = Buffer.alloc(10_000_000);
+				randomFillSync(bigModule);
+				await printBundleSize({ name: "index.js", content: "" }, [
+					{ name: "index.js", content: bigModule, type: "buffer" },
+				]);
+
+				expect(std).toMatchInlineSnapshot(`
+			Object {
+			  "debug": "",
+			  "err": "",
+			  "out": "Total Upload: xx KiB / gzip: xx KiB",
+			  "warn": "",
+			}
+		`);
+
+				process.env.NO_SCRIPT_SIZE_WARNING = previousValue;
+			});
+
+			test("should print the top biggest dependencies in the bundle when upload fails", () => {
+				const deps = {
+					"node_modules/a-mod/module.js": { bytesInOutput: 450 },
+					"node_modules/b-mod/module.js": { bytesInOutput: 10 },
+					"node_modules/c-mod/module.js": { bytesInOutput: 200 },
+					"node_modules/d-mod/module.js": { bytesInOutput: 2111200 }, // 1
+					"node_modules/e-mod/module.js": { bytesInOutput: 8209 }, // 3
+					"node_modules/f-mod/module.js": { bytesInOutput: 770 },
+					"node_modules/g-mod/module.js": { bytesInOutput: 78902 }, // 2
+					"node_modules/h-mod/module.js": { bytesInOutput: 899 },
+					"node_modules/i-mod/module.js": { bytesInOutput: 2001 }, // 4
+					"node_modules/j-mod/module.js": { bytesInOutput: 900 }, // 5
+					"node_modules/k-mod/module.js": { bytesInOutput: 79 },
+				};
+
+				printOffendingDependencies(deps);
+				expect(std).toMatchInlineSnapshot(`
+			Object {
+			  "debug": "",
+			  "err": "",
+			  "out": "",
+			  "warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mHere are the 5 largest dependencies included in your script:[0m
+
+			  - node_modules/d-mod/module.js - xx KiB
+			  - node_modules/g-mod/module.js - xx KiB
+			  - node_modules/e-mod/module.js - xx KiB
+			  - node_modules/i-mod/module.js - xx KiB
+			  - node_modules/j-mod/module.js - xx KiB
+			  If these are unnecessary, consider removing them
+
+			",
+			}
+		`);
+			});
+		});
 	});
 
 	describe("--no-bundle", () => {

diff --git a/packages/wrangler/src/bundle-reporter.tsx b/packages/wrangler/src/bundle-reporter.tsx
@@ -2,6 +2,10 @@ import { Blob } from "node:buffer";
 import { gzipSync } from "node:zlib";
 import { logger } from "./logger";
 import type { CfModule } from "./worker";
+import type { Metafile } from "esbuild";
+
+const ONE_KIB_BYTES = 1024;
+const ONE_MIB_BYTES = ONE_KIB_BYTES * 1024;
 
 async function getSize(modules: CfModule[]) {
 	const gzipSize = gzipSync(
@@ -21,9 +25,44 @@ export async function printBundleSize(
 ) {
 	const { size, gzipSize } = await getSize([...modules, main]);
 
-	const bundleReport = `${(size / 1024).toFixed(2)} KiB / gzip: ${(
-		gzipSize / 1024
+	const bundleReport = `${(size / ONE_KIB_BYTES).toFixed(2)} KiB / gzip: ${(
+		gzipSize / ONE_KIB_BYTES
 	).toFixed(2)} KiB`;
 
 	logger.log(`Total Upload: ${bundleReport}`);
+
+	if (gzipSize > ONE_MIB_BYTES && !process.env.NO_SCRIPT_SIZE_WARNING) {
+		logger.warn(
+			"We recommend keeping your script less than 1MiB (1024 KiB) after gzip. Exceeding past this can affect cold start time"
+		);
+	}
+}
+
+export function printOffendingDependencies(
+	dependencies: Metafile["outputs"][string]["inputs"]
+) {
+	const warning: string[] = [];
+
+	const dependenciesSorted = Object.entries(dependencies);
+	dependenciesSorted.sort(
+		([_adep, aData], [_bdep, bData]) =>
+			bData.bytesInOutput - aData.bytesInOutput
+	);
+	const topLargest = dependenciesSorted.slice(0, 5);
+
+	if (topLargest.length > 0) {
+		warning.push(
+			`Here are the ${topLargest.length} largest dependencies included in your script:`
+		);
+
+		for (const [dep, data] of topLargest) {
+			warning.push(
+				`- ${dep} - ${(data.bytesInOutput / ONE_KIB_BYTES).toFixed(2)} KiB`
+			);
+		}
+
+		warning.push("If these are unnecessary, consider removing them");
+
+		logger.warn(warning.join("\n"));
+	}
 }
diff --git a/packages/wrangler/src/bundle.ts b/packages/wrangler/src/bundle.ts
@@ -15,6 +15,7 @@ import type { CfModule } from "./worker";
 
 type BundleResult = {
 	modules: CfModule[];
+	dependencies: esbuild.Metafile["outputs"][string]["inputs"];
 	resolvedEntryPointPath: string;
 	bundleType: "esm" | "commonjs";
 	stop: (() => void) | undefined;
@@ -332,7 +333,8 @@ export async function bundleWorker(
 			listEntryPoints(entryPointOutputs)
 	);
 
-	const entryPointExports = entryPointOutputs[0][1].exports;
+	const { exports: entryPointExports, inputs: dependencies } =
+		entryPointOutputs[0][1];
 	const bundleType = entryPointExports.length > 0 ? "esm" : "commonjs";
 
 	const sourceMapPath = Object.keys(result.metafile.outputs).filter((_path) =>
@@ -341,6 +343,7 @@ export async function bundleWorker(
 
 	return {
 		modules: moduleCollector.modules,
+		dependencies,
 		resolvedEntryPointPath: path.resolve(
 			entry.directory,
 			entryPointOutputs[0][0]

diff --git a/packages/wrangler/src/dev/start-server.ts b/packages/wrangler/src/dev/start-server.ts
@@ -226,10 +226,12 @@ async function runEsbuild({
 		resolvedEntryPointPath,
 		bundleType,
 		modules,
+		dependencies,
 		sourceMapPath,
 	}: Awaited<ReturnType<typeof bundleWorker>> = noBundle
 		? {
 				modules: [],
+				dependencies: {},
 				resolvedEntryPointPath: entry.file,
 				bundleType: entry.format === "modules" ? "esm" : "commonjs",
 				stop: undefined,
@@ -265,6 +267,7 @@ async function runEsbuild({
 		path: resolvedEntryPointPath,
 		type: bundleType,
 		modules,
+		dependencies,
 		sourceMapPath,
 	};
 }

diff --git a/packages/wrangler/src/dev/use-esbuild.ts b/packages/wrangler/src/dev/use-esbuild.ts
@@ -8,14 +8,15 @@ import type { Config } from "../config";
 import type { WorkerRegistry } from "../dev-registry";
 import type { Entry } from "../entry";
 import type { CfModule } from "../worker";
-import type { WatchMode } from "esbuild";
+import type { WatchMode, Metafile } from "esbuild";
 
 export type EsbuildBundle = {
 	id: number;
 	path: string;
 	entry: Entry;
 	type: "esm" | "commonjs";
 	modules: CfModule[];
+	dependencies: Metafile["outputs"][string]["inputs"];
 	sourceMapPath: string | undefined;
 };
 
@@ -100,11 +101,13 @@ export function useEsbuild({
 				resolvedEntryPointPath,
 				bundleType,
 				modules,
+				dependencies,
 				stop,
 				sourceMapPath,
 			}: Awaited<ReturnType<typeof bundleWorker>> = noBundle
 				? {
 						modules: [],
+						dependencies: {},
 						resolvedEntryPointPath: entry.file,
 						bundleType: entry.format === "modules" ? "esm" : "commonjs",
 						stop: undefined,
@@ -158,6 +161,7 @@ export function useEsbuild({
 				path: resolvedEntryPointPath,
 				type: bundleType,
 				modules,
+				dependencies,
 				sourceMapPath,
 			});
 		}