From ab124d9f8d547739187fcc01a7f70afeabafc312 Mon Sep 17 00:00:00 2001 From: Will Taylor Date: Mon, 16 Dec 2024 09:22:38 -0500 Subject: [PATCH] Update based on PR feedback --- .../wrangler/src/__tests__/deploy.test.ts | 91 ++----------------- packages/wrangler/src/__tests__/dev.test.ts | 9 +- packages/wrangler/src/assets.ts | 17 +--- 3 files changed, 19 insertions(+), 98 deletions(-) diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index 1521ba35e9ebd..3868bb6a81882 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -4446,88 +4446,6 @@ addEventListener('fetch', event => {});` `); }); - it("should warn when using smart placement with assets-first", 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"); - writeWranglerConfig({ - assets: { - directory: "assets", - experimental_serve_directly: true, - }, - placement: { - mode: "smart", - }, - }); - const bodies: AssetManifest[] = []; - await mockAUSRequest(bodies); - mockSubDomainRequest(); - mockUploadWorkerRequest({ - expectedAssets: { - jwt: "<>", - config: { - serve_directly: true, - }, - }, - expectedType: "none", - }); - - await runWrangler("deploy"); - - expect(std.warn).toMatchInlineSnapshot(` - "▲ [WARNING] Using assets with smart placement turned on may result in poor performance. - - " - `); - }); - - it("should warn when using smart placement with assets-first", 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"); - writeWranglerConfig({ - assets: { - directory: "assets", - experimental_serve_directly: true, - }, - placement: { - mode: "smart", - }, - }); - const bodies: AssetManifest[] = []; - await mockAUSRequest(bodies); - mockSubDomainRequest(); - mockUploadWorkerRequest({ - expectedAssets: { - jwt: "<>", - config: { - serve_directly: true, - }, - }, - expectedType: "none", - }); - - await runWrangler("deploy"); - - expect(std.warn).toMatchInlineSnapshot(` - "▲ [WARNING] Using assets with smart placement turned on may result in poor performance. - - " - `); - }); - it("should warn if experimental_serve_directly=false but no binding is provided", async () => { const assets = [ { filePath: ".assetsignore", content: "*.bak\nsub-dir" }, @@ -4562,13 +4480,18 @@ addEventListener('fetch', event => {});` await runWrangler("deploy"); expect(std.warn).toMatchInlineSnapshot(` - "▲ [WARNING] experimental_serve_directly=false but no assets.binding provided. + "▲ [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 [assets.binding] in your configuration file. + + Read more: https://developers.cloudflare.com/workers/static-assets/binding/#binding " `); }); - it("should error if an experimental_serve_directly is false without providing a user Worker", async () => { + it("should error if experimental_serve_directly is false and no user Worker is provided", async () => { writeWranglerConfig({ assets: { directory: "xyz", diff --git a/packages/wrangler/src/__tests__/dev.test.ts b/packages/wrangler/src/__tests__/dev.test.ts index 8c801140eb437..dd4b89b49fe1d 100644 --- a/packages/wrangler/src/__tests__/dev.test.ts +++ b/packages/wrangler/src/__tests__/dev.test.ts @@ -1720,13 +1720,18 @@ describe.sequential("wrangler dev", () => { await runWranglerUntilConfig("dev"); expect(std.warn).toMatchInlineSnapshot(` - "▲ [WARNING] experimental_serve_directly=false but no assets.binding provided. + "▲ [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 [assets.binding] in your configuration file. + + Read more: https://developers.cloudflare.com/workers/static-assets/binding/#binding " `); }); - it("should error if an experimental_serve_directly is false without providing a user Worker", async () => { + it("should error if experimental_serve_directly is false and no user Worker is provided", async () => { writeWranglerConfig({ assets: { directory: "assets", experimental_serve_directly: false }, }); diff --git a/packages/wrangler/src/assets.ts b/packages/wrangler/src/assets.ts index fb939e9b1c007..998b41d1eff93 100644 --- a/packages/wrangler/src/assets.ts +++ b/packages/wrangler/src/assets.ts @@ -444,17 +444,7 @@ export function validateAssetsArgsAndConfig( ); } - // Smart placement turned on when using assets - if ( - config?.placement?.mode === "smart" && - config?.assets?.experimental_serve_directly - ) { - logger.warn( - "Using assets with smart placement turned on may result in poor performance." - ); - } - - // User worker ahead of assets, but no assets binding provided + // User Worker ahead of assets, but no assets binding provided if ( "legacy" in args ? args.assets?.assetConfig?.serve_directly === false && @@ -463,7 +453,10 @@ export function validateAssetsArgsAndConfig( !config?.assets?.binding ) { logger.warn( - "experimental_serve_directly=false but no assets.binding provided." + "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 [assets.binding] in your configuration file.\n\n" + + "Read more: https://developers.cloudflare.com/workers/static-assets/binding/#binding" ); }