Skip to content

Commit

Permalink
Update based on PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@WillTaylorDev
WillTaylorDev committed Dec 18, 2024
1 parent c2ce101 commit ab124d9
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 98 deletions.
91 changes: 7 additions & 84 deletions packages/wrangler/src/__tests__/deploy.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: "<<aus-completion-token>>",
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: "<<aus-completion-token>>",
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" },
Expand Down Expand Up @@ -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",
Expand Down
9 changes: 7 additions & 2 deletions packages/wrangler/src/__tests__/dev.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
});
Expand Down
17 changes: 5 additions & 12 deletions packages/wrangler/src/assets.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand All @@ -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"
);
}

Expand Down

0 comments on commit ab124d9

Please sign in to comment.