Skip to content

Commit 469dfb8

Browse files
WillTaylorDevWillTaylorDev
committed
Provide validation around experimental_serve_directly usage in dev and deploy
1 parent 4fe93de commit 469dfb8

File tree

4 files changed

+213
-0
lines changed

4 files changed

+213
-0
lines changed

‎.changeset/swift-zebras-guess.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+
Provide validation around assets.experimental_serve_directly

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

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4444,6 +4444,143 @@ addEventListener('fetch', event => {});`
44444444
`);
44454445
});
44464446

4447+
it("should warn when using smart placement with assets-first", async () => {
4448+
const assets = [
4449+
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
4450+
{ filePath: "file-1.txt", content: "Content of file-1" },
4451+
{ filePath: "file-2.bak", content: "Content of file-2" },
4452+
{ filePath: "file-3.txt", content: "Content of file-3" },
4453+
{ filePath: "sub-dir/file-4.bak", content: "Content of file-4" },
4454+
{ filePath: "sub-dir/file-5.txt", content: "Content of file-5" },
4455+
];
4456+
writeAssets(assets, "assets");
4457+
writeWranglerConfig({
4458+
assets: {
4459+
directory: "assets",
4460+
experimental_serve_directly: true,
4461+
},
4462+
placement: {
4463+
mode: "smart",
4464+
},
4465+
});
4466+
const bodies: AssetManifest[] = [];
4467+
await mockAUSRequest(bodies);
4468+
mockSubDomainRequest();
4469+
mockUploadWorkerRequest({
4470+
expectedAssets: {
4471+
jwt: "<<aus-completion-token>>",
4472+
config: {
4473+
serve_directly: true,
4474+
},
4475+
},
4476+
expectedType: "none",
4477+
});
4478+
4479+
await runWrangler("deploy");
4480+
4481+
expect(std.warn).toMatchInlineSnapshot(`
4482+
"▲ [WARNING] Using assets with smart placement turned on may result in poor performance.
4483+
4484+
"
4485+
`);
4486+
});
4487+
4488+
it("should warn when using smart placement with assets-first", async () => {
4489+
const assets = [
4490+
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
4491+
{ filePath: "file-1.txt", content: "Content of file-1" },
4492+
{ filePath: "file-2.bak", content: "Content of file-2" },
4493+
{ filePath: "file-3.txt", content: "Content of file-3" },
4494+
{ filePath: "sub-dir/file-4.bak", content: "Content of file-4" },
4495+
{ filePath: "sub-dir/file-5.txt", content: "Content of file-5" },
4496+
];
4497+
writeAssets(assets, "assets");
4498+
writeWranglerConfig({
4499+
assets: {
4500+
directory: "assets",
4501+
experimental_serve_directly: true,
4502+
},
4503+
placement: {
4504+
mode: "smart",
4505+
},
4506+
});
4507+
const bodies: AssetManifest[] = [];
4508+
await mockAUSRequest(bodies);
4509+
mockSubDomainRequest();
4510+
mockUploadWorkerRequest({
4511+
expectedAssets: {
4512+
jwt: "<<aus-completion-token>>",
4513+
config: {
4514+
serve_directly: true,
4515+
},
4516+
},
4517+
expectedType: "none",
4518+
});
4519+
4520+
await runWrangler("deploy");
4521+
4522+
expect(std.warn).toMatchInlineSnapshot(`
4523+
"▲ [WARNING] Using assets with smart placement turned on may result in poor performance.
4524+
4525+
"
4526+
`);
4527+
});
4528+
4529+
it("should warn if experimental_serve_directly=false but no binding is provided", async () => {
4530+
const assets = [
4531+
{ filePath: ".assetsignore", content: "*.bak\nsub-dir" },
4532+
{ filePath: "file-1.txt", content: "Content of file-1" },
4533+
{ filePath: "file-2.bak", content: "Content of file-2" },
4534+
{ filePath: "file-3.txt", content: "Content of file-3" },
4535+
{ filePath: "sub-dir/file-4.bak", content: "Content of file-4" },
4536+
{ filePath: "sub-dir/file-5.txt", content: "Content of file-5" },
4537+
];
4538+
writeAssets(assets, "assets");
4539+
writeWorkerSource({ format: "js" });
4540+
writeWranglerConfig({
4541+
main: "index.js",
4542+
assets: {
4543+
directory: "assets",
4544+
experimental_serve_directly: false,
4545+
},
4546+
});
4547+
const bodies: AssetManifest[] = [];
4548+
await mockAUSRequest(bodies);
4549+
mockSubDomainRequest();
4550+
mockUploadWorkerRequest({
4551+
expectedAssets: {
4552+
jwt: "<<aus-completion-token>>",
4553+
config: {
4554+
serve_directly: false,
4555+
},
4556+
},
4557+
expectedMainModule: "index.js",
4558+
});
4559+
4560+
await runWrangler("deploy");
4561+
4562+
expect(std.warn).toMatchInlineSnapshot(`
4563+
"▲ [WARNING] experimental_serve_directly=false but no assets.binding provided.
4564+
4565+
"
4566+
`);
4567+
});
4568+
4569+
it("should error if an experimental_serve_directly is false without providing a user Worker", async () => {
4570+
writeWranglerConfig({
4571+
assets: {
4572+
directory: "xyz",
4573+
experimental_serve_directly: false,
4574+
},
4575+
});
4576+
4577+
await expect(runWrangler("deploy")).rejects
4578+
.toThrowErrorMatchingInlineSnapshot(`
4579+
[Error: Cannot set experimental_serve_directly=false without a Worker script.
4580+
Please remove experimental_serve_directly from your configuration file, or provide a Worker script in your configuration file (\`main\`).]
4581+
`);
4582+
});
4583+
44474584
it("should be able to upload files with special characters in filepaths", async () => {
44484585
// NB windows will disallow these characters in file paths anyway < > : " / \ | ? *
44494586
const assets = [

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,6 +1706,41 @@ describe.sequential("wrangler dev", () => {
17061706
);
17071707
});
17081708

1709+
it("should warn if experimental_serve_directly=false but no binding is provided", async () => {
1710+
writeWranglerConfig({
1711+
main: "index.js",
1712+
assets: {
1713+
directory: "assets",
1714+
experimental_serve_directly: false,
1715+
},
1716+
});
1717+
fs.mkdirSync("assets");
1718+
fs.writeFileSync("index.js", `export default {};`);
1719+
1720+
await runWranglerUntilConfig("dev");
1721+
1722+
expect(std.warn).toMatchInlineSnapshot(`
1723+
"▲ [WARNING] experimental_serve_directly=false but no assets.binding provided.
1724+
1725+
"
1726+
`);
1727+
});
1728+
1729+
it("should error if an experimental_serve_directly is false without providing a user Worker", async () => {
1730+
writeWranglerConfig({
1731+
assets: { directory: "assets", experimental_serve_directly: false },
1732+
});
1733+
fs.mkdirSync("assets");
1734+
await expect(
1735+
runWrangler("dev")
1736+
).rejects.toThrowErrorMatchingInlineSnapshot(
1737+
`
1738+
[Error: Cannot set experimental_serve_directly=false without a Worker script.
1739+
Please remove experimental_serve_directly from your configuration file, or provide a Worker script in your configuration file (\`main\`).]
1740+
`
1741+
);
1742+
});
1743+
17091744
it("should error if directory specified by '--assets' command line argument does not exist", async () => {
17101745
writeWranglerConfig({
17111746
main: "./index.js",

‎packages/wrangler/src/assets.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,42 @@ export function validateAssetsArgsAndConfig(
443443
"Please remove the asset binding from your configuration file, or provide a Worker script in your configuration file (`main`)."
444444
);
445445
}
446+
447+
// Smart placement turned on when using assets
448+
if (
449+
config?.placement?.mode === "smart" &&
450+
config?.assets?.experimental_serve_directly
451+
) {
452+
logger.warn(
453+
"Using assets with smart placement turned on may result in poor performance."
454+
);
455+
}
456+
457+
// User worker ahead of assets, but no assets binding provided
458+
if (
459+
"legacy" in args
460+
? args.assets?.assetConfig?.serve_directly === false &&
461+
!args.assets?.binding
462+
: config?.assets?.experimental_serve_directly === false &&
463+
!config?.assets?.binding
464+
) {
465+
logger.warn(
466+
"experimental_serve_directly=false but no assets.binding provided."
467+
);
468+
}
469+
470+
// Using serve_directly=false, but didn't provide a Worker script
471+
if (
472+
"legacy" in args
473+
? args.entrypoint === noOpEntrypoint &&
474+
args.assets?.assetConfig?.serve_directly === false
475+
: !config?.main && config?.assets?.experimental_serve_directly === false
476+
) {
477+
throw new UserError(
478+
"Cannot set experimental_serve_directly=false without a Worker script.\n" +
479+
"Please remove experimental_serve_directly from your configuration file, or provide a Worker script in your configuration file (`main`)."
480+
);
481+
}
446482
}
447483

448484
const CF_ASSETS_IGNORE_FILENAME = ".assetsignore";

0 commit comments

Comments
 (0)