From 726b87fde70540c67241f4fbb060a64e22648826 Mon Sep 17 00:00:00 2001 From: WillTaylorDev Date: Thu, 19 Dec 2024 05:09:03 -0500 Subject: [PATCH] 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 --- .changeset/swift-zebras-guess.md | 5 ++ .../wrangler/src/__tests__/deploy.test.ts | 61 +++++++++++++++++++ packages/wrangler/src/__tests__/dev.test.ts | 41 +++++++++++++ packages/wrangler/src/assets.ts | 29 +++++++++ 4 files changed, 136 insertions(+) create mode 100644 .changeset/swift-zebras-guess.md diff --git a/.changeset/swift-zebras-guess.md b/.changeset/swift-zebras-guess.md new file mode 100644 index 0000000000000..91e2232cc5776 --- /dev/null +++ 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 index 9c8089be5392f..e74ab2ee5282a 100644 --- 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: "<>", + config: { + serve_directly: false, + }, + }, + expectedMainModule: "index.js", + }); + + await runWrangler("deploy"); + + expect(std.warn).toMatchInlineSnapshot(` + "▲ [WARNING] experimental_serve_directly=false set without an assets binding + + 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: https://developers.cloudflare.com/workers/static-assets/binding/#binding + + " + `); + }); + + 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 index 23d77e47ea011..a3d063658acb1 100644 --- 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(` + "▲ [WARNING] experimental_serve_directly=false set without an assets binding + + 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: https://developers.cloudflare.com/workers/static-assets/binding/#binding + + " + `); + }); + + 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 index 999f2dca6f52b..f3f8802e08916 100644 --- 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";