Skip to content

Commit 79c1528

Browse files
WillTaylorDevCarmenPopoviciu
authored andcommitted
Update based on PR feedback
1 parent c2ce101 commit 79c1528

File tree

3 files changed

+34
-52
lines changed

3 files changed

+34
-52
lines changed

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

Lines changed: 18 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4446,7 +4446,7 @@ addEventListener('fetch', event => {});`
44464446
`);
44474447
});
44484448

4449-
it("should warn when using smart placement with assets-first", async () => {
4449+
it("should warn when using smart placement with Worker first", async () => {
44504450
const assets = [
44514451
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
44524452
{ filePath: "file-1.txt", content: "Content of file-1" },
@@ -4456,10 +4456,13 @@ addEventListener('fetch', event => {});`
44564456
{ filePath: "sub-dir/file-5.txt", content: "Content of file-5" },
44574457
];
44584458
writeAssets(assets, "assets");
4459+
writeWorkerSource({ format: "js" });
44594460
writeWranglerConfig({
4461+
main: "index.js",
44604462
assets: {
44614463
directory: "assets",
4462-
experimental_serve_directly: true,
4464+
experimental_serve_directly: false,
4465+
binding: "ASSETS",
44634466
},
44644467
placement: {
44654468
mode: "smart",
@@ -4472,57 +4475,21 @@ addEventListener('fetch', event => {});`
44724475
expectedAssets: {
44734476
jwt: "<<aus-completion-token>>",
44744477
config: {
4475-
serve_directly: true,
4478+
serve_directly: false,
44764479
},
44774480
},
4478-
expectedType: "none",
4481+
expectedMainModule: "index.js",
4482+
expectedBindings: [{ name: "ASSETS", type: "assets" }],
44794483
});
44804484

44814485
await runWrangler("deploy");
44824486

44834487
expect(std.warn).toMatchInlineSnapshot(`
4484-
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mUsing assets with smart placement turned on may result in poor performance.[0m
4488+
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mTurning on Smart Placement in a Worker that is using assets and serve_directlyset tofalse 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.[0m
44854489
4486-
"
4487-
`);
4488-
});
4490+
This could result in poor performance as round trip times could increase when serving assets.
44894491
4490-
it("should warn when using smart placement with assets-first", async () => {
4491-
const assets = [
4492-
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
4493-
{ filePath: "file-1.txt", content: "Content of file-1" },
4494-
{ filePath: "file-2.bak", content: "Content of file-2" },
4495-
{ filePath: "file-3.txt", content: "Content of file-3" },
4496-
{ filePath: "sub-dir/file-4.bak", content: "Content of file-4" },
4497-
{ filePath: "sub-dir/file-5.txt", content: "Content of file-5" },
4498-
];
4499-
writeAssets(assets, "assets");
4500-
writeWranglerConfig({
4501-
assets: {
4502-
directory: "assets",
4503-
experimental_serve_directly: true,
4504-
},
4505-
placement: {
4506-
mode: "smart",
4507-
},
4508-
});
4509-
const bodies: AssetManifest[] = [];
4510-
await mockAUSRequest(bodies);
4511-
mockSubDomainRequest();
4512-
mockUploadWorkerRequest({
4513-
expectedAssets: {
4514-
jwt: "<<aus-completion-token>>",
4515-
config: {
4516-
serve_directly: true,
4517-
},
4518-
},
4519-
expectedType: "none",
4520-
});
4521-
4522-
await runWrangler("deploy");
4523-
4524-
expect(std.warn).toMatchInlineSnapshot(`
4525-
"▲ [WARNING] Using assets with smart placement turned on may result in poor performance.
4492+
Read more: https://developers.cloudflare.com/workers/static-assets/binding/#smart-placement
45264493
45274494
"
45284495
`);
@@ -4562,13 +4529,18 @@ addEventListener('fetch', event => {});`
45624529
await runWrangler("deploy");
45634530

45644531
expect(std.warn).toMatchInlineSnapshot(`
4565-
"▲ [WARNING] experimental_serve_directly=false but no assets.binding provided.
4532+
"▲ [WARNING] Assets are not accessible, as the [assets.experimental_serve_directly] configuration key is set to \`false\`, but no [assets.binding] configuration is provided.
4533+
4534+
❯❯ Add an assets binding to your assets configuration: binding = \\"ASSETS\\", or
4535+
❯❯ Set experimental_serve_directly=false in your assets configuration
4536+
4537+
Read more: https://developers.cloudflare.com/workers/static-assets/binding/#binding
45664538
45674539
"
45684540
`);
45694541
});
45704542

4571-
it("should error if an experimental_serve_directly is false without providing a user Worker", async () => {
4543+
it("should error if experimental_serve_directly is false and no user Worker is provided", async () => {
45724544
writeWranglerConfig({
45734545
assets: {
45744546
directory: "xyz",

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1720,13 +1720,18 @@ describe.sequential("wrangler dev", () => {
17201720
await runWranglerUntilConfig("dev");
17211721

17221722
expect(std.warn).toMatchInlineSnapshot(`
1723-
"▲ [WARNING] experimental_serve_directly=false but no assets.binding provided.
1723+
"▲ [WARNING] Assets are not accessible, as the [assets.experimental_serve_directly] configuration key is set to \`false\`, but no [assets.binding] configuration is provided.
1724+
1725+
❯❯ Add an assets binding to your assets configuration: binding = \\"ASSETS\\", or
1726+
❯❯ Set experimental_serve_directly=false in your assets configuration
1727+
1728+
Read more: https://developers.cloudflare.com/workers/static-assets/binding/#binding
17241729
17251730
"
17261731
`);
17271732
});
17281733

1729-
it("should error if an experimental_serve_directly is false without providing a user Worker", async () => {
1734+
it("should error if experimental_serve_directly is false and no user Worker is provided", async () => {
17301735
writeWranglerConfig({
17311736
assets: { directory: "assets", experimental_serve_directly: false },
17321737
});

packages/wrangler/src/assets.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -447,14 +447,16 @@ export function validateAssetsArgsAndConfig(
447447
// Smart placement turned on when using assets
448448
if (
449449
config?.placement?.mode === "smart" &&
450-
config?.assets?.experimental_serve_directly
450+
config?.assets?.experimental_serve_directly === false
451451
) {
452452
logger.warn(
453-
"Using assets with smart placement turned on may result in poor performance."
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"
454456
);
455457
}
456458

457-
// User worker ahead of assets, but no assets binding provided
459+
// User Worker ahead of assets, but no assets binding provided
458460
if (
459461
"legacy" in args
460462
? args.assets?.assetConfig?.serve_directly === false &&
@@ -463,7 +465,10 @@ export function validateAssetsArgsAndConfig(
463465
!config?.assets?.binding
464466
) {
465467
logger.warn(
466-
"experimental_serve_directly=false but no assets.binding provided."
468+
"Assets are not accessible, as the [assets.experimental_serve_directly] configuration key is set to `false`, but no [assets.binding] configuration is provided.\n" +
469+
'❯❯ Add an assets binding to your assets configuration: binding = "ASSETS", or\n' +
470+
"❯❯ Set experimental_serve_directly=false in your assets configuration\n\n" +
471+
"Read more: https://developers.cloudflare.com/workers/static-assets/binding/#binding"
467472
);
468473
}
469474

0 commit comments

Comments
 (0)