Skip to content

Commit e1f1e05

Browse files
WillTaylorDevpenalosa
authored andcommitted
Add warning for smart placement w/assets (#7568)
* Provide validation around experimental_serve_directly usage in dev and deploy * Update based on PR feedback
1 parent 756065c commit e1f1e05

File tree

3 files changed

+66
-0
lines changed

3 files changed

+66
-0
lines changed

.changeset/tidy-kiwis-arrive.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": minor
3+
---
4+
5+
Warn users when using smart placement with Workers + Assets and `serve_directly` is set to `false`

packages/wrangler/src/__tests__/deploy.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4687,6 +4687,55 @@ addEventListener('fetch', event => {});`
46874687
`);
46884688
});
46894689

4690+
it("should warn when using smart placement with Worker first", async () => {
4691+
const assets = [
4692+
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
4693+
{ filePath: "file-1.txt", content: "Content of file-1" },
4694+
{ filePath: "file-2.bak", content: "Content of file-2" },
4695+
{ filePath: "file-3.txt", content: "Content of file-3" },
4696+
{ filePath: "sub-dir/file-4.bak", content: "Content of file-4" },
4697+
{ filePath: "sub-dir/file-5.txt", content: "Content of file-5" },
4698+
];
4699+
writeAssets(assets, "assets");
4700+
writeWorkerSource({ format: "js" });
4701+
writeWranglerConfig({
4702+
main: "index.js",
4703+
assets: {
4704+
directory: "assets",
4705+
experimental_serve_directly: false,
4706+
binding: "ASSETS",
4707+
},
4708+
placement: {
4709+
mode: "smart",
4710+
},
4711+
});
4712+
const bodies: AssetManifest[] = [];
4713+
await mockAUSRequest(bodies);
4714+
mockSubDomainRequest();
4715+
mockUploadWorkerRequest({
4716+
expectedAssets: {
4717+
jwt: "<<aus-completion-token>>",
4718+
config: {
4719+
serve_directly: false,
4720+
},
4721+
},
4722+
expectedMainModule: "index.js",
4723+
expectedBindings: [{ name: "ASSETS", type: "assets" }],
4724+
});
4725+
4726+
await runWrangler("deploy");
4727+
4728+
expect(std.warn).toMatchInlineSnapshot(`
4729+
"▲ [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.
4730+
4731+
This could result in poor performance as round trip times could increase when serving assets.
4732+
4733+
Read more: https://developers.cloudflare.com/workers/static-assets/binding/#smart-placement
4734+
4735+
"
4736+
`);
4737+
});
4738+
46904739
it("should warn if experimental_serve_directly=false but no binding is provided", async () => {
46914740
const assets = [
46924741
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },

packages/wrangler/src/assets.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,18 @@ export function validateAssetsArgsAndConfig(
444444
);
445445
}
446446

447+
// Smart placement turned on when using assets
448+
if (
449+
config?.placement?.mode === "smart" &&
450+
config?.assets?.experimental_serve_directly === false
451+
) {
452+
logger.warn(
453+
"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" +
454+
"This could result in poor performance as round trip times could increase when serving assets.\n\n" +
455+
"Read more: https://developers.cloudflare.com/workers/static-assets/binding/#smart-placement"
456+
);
457+
}
458+
447459
// User Worker ahead of assets, but no assets binding provided
448460
if (
449461
"legacy" in args

0 commit comments

Comments
 (0)