Provide validation around experimental_serve_directly usage (#7537)

* Provide validation around experimental_serve_directly usage in dev and deploy * Update based on PR feedback * Update packages/wrangler/src/assets.ts * Update packages/wrangler/src/__tests__/dev.test.ts * Update packages/wrangler/src/__tests__/deploy.test.ts * fix --------- Co-authored-by: Carmen Popoviciu <cpopoviciu@cloudflare.com>
cloudflare · Jan 10, 2025 · 726b87f · 726b87f
1 parent 9b982ba
commit 726b87f
Show file tree

Hide file tree

Showing 4 changed files with 136 additions and 0 deletions.
diff --git a/.changeset/swift-zebras-guess.md b/.changeset/swift-zebras-guess.md
@@ -0,0 +1,5 @@
+---
+"wrangler": minor
+---
+
+Provide validation around assets.experimental_serve_directly
diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts
@@ -4687,6 +4687,67 @@ addEventListener('fetch', event => {});`
 			`);
 		});
 
+		it("should warn if experimental_serve_directly=false but no binding is provided", async () => {
+			const assets = [
+				{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
+				{ filePath: "file-1.txt", content: "Content of file-1" },
+				{ filePath: "file-2.bak", content: "Content of file-2" },
+				{ filePath: "file-3.txt", content: "Content of file-3" },
+				{ filePath: "sub-dir/file-4.bak", content: "Content of file-4" },
+				{ filePath: "sub-dir/file-5.txt", content: "Content of file-5" },
+			];
+			writeAssets(assets, "assets");
+			writeWorkerSource({ format: "js" });
+			writeWranglerConfig({
+				main: "index.js",
+				assets: {
+					directory: "assets",
+					experimental_serve_directly: false,
+				},
+			});
+			const bodies: AssetManifest[] = [];
+			await mockAUSRequest(bodies);
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({
+				expectedAssets: {
+					jwt: "<<aus-completion-token>>",
+					config: {
+						serve_directly: false,
+					},
+				},
+				expectedMainModule: "index.js",
+			});
+
+			await runWrangler("deploy");
+
+			expect(std.warn).toMatchInlineSnapshot(`
+				"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mexperimental_serve_directly=false set without an assets binding[0m
+
+				  Setting experimental_serve_directly to false will always invoke your Worker script.
+				  To fetch your assets from your Worker, please set the [assets.binding] key in your configuration
+				  file.
+
+				  Read more: [4mhttps://developers.cloudflare.com/workers/static-assets/binding/#binding[0m
+
+				"
+			`);
+		});
+
+		it("should error if experimental_serve_directly is false and no user Worker is provided", async () => {
+			writeWranglerConfig({
+				assets: {
+					directory: "xyz",
+					experimental_serve_directly: false,
+				},
+			});
+
+			await expect(runWrangler("deploy")).rejects
+				.toThrowErrorMatchingInlineSnapshot(`
+				[Error: Cannot set experimental_serve_directly=false without a Worker script.
+				Please remove experimental_serve_directly from your configuration file, or provide a Worker script in your configuration file (\`main\`).]
+			`);
+		});
+
 		it("should be able to upload files with special characters in filepaths", async () => {
 			// NB windows will disallow these characters in file paths anyway < > : " / \ | ? *
 			const assets = [

diff --git a/packages/wrangler/src/__tests__/dev.test.ts b/packages/wrangler/src/__tests__/dev.test.ts
@@ -1679,6 +1679,47 @@ describe.sequential("wrangler dev", () => {
 			);
 		});
 
+		it("should warn if experimental_serve_directly=false but no binding is provided", async () => {
+			writeWranglerConfig({
+				main: "index.js",
+				assets: {
+					directory: "assets",
+					experimental_serve_directly: false,
+				},
+			});
+			fs.mkdirSync("assets");
+			fs.writeFileSync("index.js", `export default {};`);
+
+			await runWranglerUntilConfig("dev");
+
+			expect(std.warn).toMatchInlineSnapshot(`
+				"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mexperimental_serve_directly=false set without an assets binding[0m
+
+				  Setting experimental_serve_directly to false will always invoke your Worker script.
+				  To fetch your assets from your Worker, please set the [assets.binding] key in your configuration
+				  file.
+
+				  Read more: [4mhttps://developers.cloudflare.com/workers/static-assets/binding/#binding[0m
+
+				"
+			`);
+		});
+
+		it("should error if experimental_serve_directly is false and no user Worker is provided", async () => {
+			writeWranglerConfig({
+				assets: { directory: "assets", experimental_serve_directly: false },
+			});
+			fs.mkdirSync("assets");
+			await expect(
+				runWrangler("dev")
+			).rejects.toThrowErrorMatchingInlineSnapshot(
+				`
+				[Error: Cannot set experimental_serve_directly=false without a Worker script.
+				Please remove experimental_serve_directly from your configuration file, or provide a Worker script in your configuration file (\`main\`).]
+			`
+			);
+		});
+
 		it("should error if directory specified by '--assets' command line argument does not exist", async () => {
 			writeWranglerConfig({
 				main: "./index.js",

diff --git a/packages/wrangler/src/assets.ts b/packages/wrangler/src/assets.ts
@@ -443,6 +443,35 @@ export function validateAssetsArgsAndConfig(
 				"Please remove the asset binding from your configuration file, or provide a Worker script in your configuration file (`main`)."
 		);
 	}
+
+	// User Worker ahead of assets, but no assets binding provided
+	if (
+		"legacy" in args
+			? args.assets?.assetConfig?.serve_directly === false &&
+				!args.assets?.binding
+			: config?.assets?.experimental_serve_directly === false &&
+				!config?.assets?.binding
+	) {
+		logger.warn(
+			"experimental_serve_directly=false set without an assets binding\n" +
+				"Setting experimental_serve_directly to false will always invoke your Worker script.\n" +
+				"To fetch your assets from your Worker, please set the [assets.binding] key in your configuration file.\n\n" +
+				"Read more: https://developers.cloudflare.com/workers/static-assets/binding/#binding"
+		);
+	}
+
+	// Using serve_directly=false, but didn't provide a Worker script
+	if (
+		"legacy" in args
+			? args.entrypoint === noOpEntrypoint &&
+				args.assets?.assetConfig?.serve_directly === false
+			: !config?.main && config?.assets?.experimental_serve_directly === false
+	) {
+		throw new UserError(
+			"Cannot set experimental_serve_directly=false without a Worker script.\n" +
+				"Please remove experimental_serve_directly from your configuration file, or provide a Worker script in your configuration file (`main`)."
+		);
+	}
 }
 
 const CF_ASSETS_IGNORE_FILENAME = ".assetsignore";