Skip to content

Commit

Permalink
Add warning for smart placement w/assets (#7568)
Browse files Browse the repository at this point in the history
* Provide validation around experimental_serve_directly usage in dev and deploy

* Update based on PR feedback
  • Loading branch information
WillTaylorDev authored Dec 19, 2024
1 parent 004fd33 commit 2bbcb93
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/tidy-kiwis-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": minor
---

Warn users when using smart placement with Workers + Assets and `serve_directly` is set to `false`
49 changes: 49 additions & 0 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4687,6 +4687,55 @@ addEventListener('fetch', event => {});`
`);
});

it("should warn when using smart placement with Worker 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");
writeWorkerSource({ format: "js" });
writeWranglerConfig({
main: "index.js",
assets: {
directory: "assets",
experimental_serve_directly: false,
binding: "ASSETS",
},
placement: {
mode: "smart",
},
});
const bodies: AssetManifest[] = [];
await mockAUSRequest(bodies);
mockSubDomainRequest();
mockUploadWorkerRequest({
expectedAssets: {
jwt: "<<aus-completion-token>>",
config: {
serve_directly: false,
},
},
expectedMainModule: "index.js",
expectedBindings: [{ name: "ASSETS", type: "assets" }],
});

await runWrangler("deploy");

expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] Turning on Smart Placement in a Worker that is using assets and serve_directly set to false means that your entire Worker could be moved to run closer to your data source, and all requests will go to that Worker before serving assets.
This could result in poor performance as round trip times could increase when serving assets.
Read more: https://developers.cloudflare.com/workers/static-assets/binding/#smart-placement
"
`);
});

it("should warn if experimental_serve_directly=false but no binding is provided", async () => {
const assets = [
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
Expand Down
12 changes: 12 additions & 0 deletions packages/wrangler/src/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,18 @@ export function validateAssetsArgsAndConfig(
);
}

// Smart placement turned on when using assets
if (
config?.placement?.mode === "smart" &&
config?.assets?.experimental_serve_directly === false
) {
logger.warn(
"Turning on Smart Placement in a Worker that is using assets and serve_directly set to false means that your entire Worker could be moved to run closer to your data source, and all requests will go to that Worker before serving assets.\n" +
"This could result in poor performance as round trip times could increase when serving assets.\n\n" +
"Read more: https://developers.cloudflare.com/workers/static-assets/binding/#smart-placement"
);
}

// User Worker ahead of assets, but no assets binding provided
if (
"legacy" in args
Expand Down

0 comments on commit 2bbcb93

Please sign in to comment.