From 32e7ba110d71e91190bdea7194850d33df7d28fa Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 26 Nov 2024 19:23:24 +0000 Subject: [PATCH] =?UTF-8?q?Revert=20"Revert=20"Add=20serve=5Fdirectly=20op?= =?UTF-8?q?tion=20to=20assets=20configuration=20(#7316)"=20(#=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit cc93b2cbdce444bc3e66916aaab1752c87fc4608. --- .changeset/six-coats-rule.md | 7 ++ fixtures/asset-config/html-handling.test.ts | 1 + fixtures/asset-config/redirects.test.ts | 3 +- .../asset-config/url-normalization.test.ts | 3 +- .../asset-worker/src/configuration.ts | 1 + .../asset-worker/tests/handler.test.ts | 4 + packages/workers-shared/utils/types.ts | 1 + .../wrangler/src/__tests__/deploy.test.ts | 112 ++++++++++++++++++ packages/wrangler/src/assets.ts | 1 + packages/wrangler/src/config/environment.ts | 1 + packages/wrangler/src/config/validation.ts | 10 ++ .../create-worker-upload-form.ts | 1 + 12 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 .changeset/six-coats-rule.md diff --git a/.changeset/six-coats-rule.md b/.changeset/six-coats-rule.md new file mode 100644 index 000000000000..4112b8521f3c --- /dev/null +++ b/.changeset/six-coats-rule.md @@ -0,0 +1,7 @@ +--- +"wrangler": minor +--- + +feat: add `serve_directly` option to Workers with Assets + +Users can now specify whether their assets are served directly against HTTP requests or whether these requests always go to the Worker, which can then respond with asset retrieved by its assets binding. diff --git a/fixtures/asset-config/html-handling.test.ts b/fixtures/asset-config/html-handling.test.ts index e024bfd956cc..2700ad69876e 100644 --- a/fixtures/asset-config/html-handling.test.ts +++ b/fixtures/asset-config/html-handling.test.ts @@ -64,6 +64,7 @@ describe.each(testSuites)("$title", ({ title, suite }) => { return { html_handling, not_found_handling: "none", + serve_directly: true, }; }); }); diff --git a/fixtures/asset-config/redirects.test.ts b/fixtures/asset-config/redirects.test.ts index 5d724394e6c2..1c8bfbbb02c2 100644 --- a/fixtures/asset-config/redirects.test.ts +++ b/fixtures/asset-config/redirects.test.ts @@ -20,7 +20,7 @@ const existsMock = (fileList: Set) => { }; const BASE_URL = "http://example.com"; -describe("[Asset Worker] `test url normalization`", () => { +describe("[Asset Worker] `test location rewrite`", () => { afterEach(() => { vi.mocked(getAssetWithMetadataFromKV).mockRestore(); }); @@ -41,6 +41,7 @@ describe("[Asset Worker] `test url normalization`", () => { return { html_handling: "none", not_found_handling: "none", + serve_directly: true, }; }); }); diff --git a/fixtures/asset-config/url-normalization.test.ts b/fixtures/asset-config/url-normalization.test.ts index c8c716b35f54..a679445e795f 100644 --- a/fixtures/asset-config/url-normalization.test.ts +++ b/fixtures/asset-config/url-normalization.test.ts @@ -20,7 +20,7 @@ const existsMock = (fileList: Set) => { }; const BASE_URL = "http://example.com"; -describe("[Asset Worker] `test redirects`", () => { +describe("[Asset Worker] `test slash normalization`", () => { afterEach(() => { vi.mocked(getAssetWithMetadataFromKV).mockRestore(); }); @@ -41,6 +41,7 @@ describe("[Asset Worker] `test redirects`", () => { return { html_handling: "none", not_found_handling: "none", + serve_directly: true, }; }); }); diff --git a/packages/workers-shared/asset-worker/src/configuration.ts b/packages/workers-shared/asset-worker/src/configuration.ts index 0df3cc7962a9..4d776715bce3 100644 --- a/packages/workers-shared/asset-worker/src/configuration.ts +++ b/packages/workers-shared/asset-worker/src/configuration.ts @@ -6,5 +6,6 @@ export const applyConfigurationDefaults = ( return { html_handling: configuration?.html_handling ?? "auto-trailing-slash", not_found_handling: configuration?.not_found_handling ?? "none", + serve_directly: configuration?.serve_directly ?? true, }; }; diff --git a/packages/workers-shared/asset-worker/tests/handler.test.ts b/packages/workers-shared/asset-worker/tests/handler.test.ts index 5f31c39a14b6..9d01d635b071 100644 --- a/packages/workers-shared/asset-worker/tests/handler.test.ts +++ b/packages/workers-shared/asset-worker/tests/handler.test.ts @@ -7,6 +7,7 @@ describe("[Asset Worker] `handleRequest`", () => { const configuration: Required = { html_handling: "none", not_found_handling: "none", + serve_directly: true, }; const eTag = "some-etag"; const exists = vi.fn().mockReturnValue(eTag); @@ -29,6 +30,7 @@ describe("[Asset Worker] `handleRequest`", () => { const configuration: Required = { html_handling: "none", not_found_handling: "none", + serve_directly: true, }; const eTag = "some-etag"; const exists = vi.fn().mockReturnValue(eTag); @@ -53,6 +55,7 @@ describe("[Asset Worker] `handleRequest`", () => { const configuration: Required = { html_handling: "none", not_found_handling: "none", + serve_directly: true, }; const eTag = "some-etag"; const exists = vi.fn().mockReturnValue(eTag); @@ -77,6 +80,7 @@ describe("[Asset Worker] `handleRequest`", () => { const configuration: Required = { html_handling: "none", not_found_handling: "none", + serve_directly: true, }; const eTag = "some-etag"; const exists = vi.fn().mockReturnValue(eTag); diff --git a/packages/workers-shared/utils/types.ts b/packages/workers-shared/utils/types.ts index c415d13c0b7c..eab59ca25767 100644 --- a/packages/workers-shared/utils/types.ts +++ b/packages/workers-shared/utils/types.ts @@ -17,6 +17,7 @@ export const AssetConfigSchema = z.object({ not_found_handling: z .enum(["single-page-application", "404-page", "none"]) .optional(), + serve_directly: z.boolean().optional(), }); export type RoutingConfig = z.infer; diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index 98271c42f4b8..c99ddd44e334 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -4948,6 +4948,118 @@ addEventListener('fetch', event => {});` await runWrangler("deploy"); }); + it("serve_directly correctly overrides default if set to false", async () => { + const assets = [ + { filePath: "file-1.txt", content: "Content of file-1" }, + { filePath: "boop/file-2.txt", content: "Content of file-2" }, + ]; + writeAssets(assets); + writeWorkerSource({ format: "js" }); + writeWranglerConfig({ + main: "index.js", + compatibility_date: "2024-09-27", + compatibility_flags: ["nodejs_compat"], + assets: { + directory: "assets", + binding: "ASSETS", + html_handling: "none", + not_found_handling: "404-page", + serve_directly: false, + }, + }); + await mockAUSRequest(); + mockSubDomainRequest(); + mockUploadWorkerRequest({ + expectedAssets: { + jwt: "<>", + config: { + html_handling: "none", + not_found_handling: "404-page", + serve_directly: false, + }, + }, + expectedBindings: [{ name: "ASSETS", type: "assets" }], + expectedMainModule: "index.js", + expectedCompatibilityDate: "2024-09-27", + expectedCompatibilityFlags: ["nodejs_compat"], + }); + await runWrangler("deploy"); + }); + + it("serve_directly omitted when not provided in config", async () => { + const assets = [ + { filePath: "file-1.txt", content: "Content of file-1" }, + { filePath: "boop/file-2.txt", content: "Content of file-2" }, + ]; + writeAssets(assets); + writeWorkerSource({ format: "js" }); + writeWranglerConfig({ + main: "index.js", + compatibility_date: "2024-09-27", + compatibility_flags: ["nodejs_compat"], + assets: { + directory: "assets", + binding: "ASSETS", + html_handling: "none", + not_found_handling: "404-page", + }, + }); + await mockAUSRequest(); + mockSubDomainRequest(); + mockUploadWorkerRequest({ + expectedAssets: { + jwt: "<>", + config: { + html_handling: "none", + not_found_handling: "404-page", + }, + }, + expectedBindings: [{ name: "ASSETS", type: "assets" }], + expectedMainModule: "index.js", + expectedCompatibilityDate: "2024-09-27", + expectedCompatibilityFlags: ["nodejs_compat"], + }); + await runWrangler("deploy"); + }); + + it("serve_directly provided if set to true", async () => { + const assets = [ + { filePath: "file-1.txt", content: "Content of file-1" }, + { filePath: "boop/file-2.txt", content: "Content of file-2" }, + ]; + writeAssets(assets); + writeWorkerSource({ format: "js" }); + writeWranglerConfig({ + main: "index.js", + compatibility_date: "2024-09-27", + compatibility_flags: ["nodejs_compat"], + assets: { + directory: "assets", + binding: "ASSETS", + html_handling: "none", + not_found_handling: "404-page", + serve_directly: true, + }, + }); + await mockAUSRequest(); + mockSubDomainRequest(); + mockUploadWorkerRequest({ + expectedAssets: { + jwt: "<>", + config: { + html_handling: "none", + not_found_handling: "404-page", + serve_directly: true, + }, + }, + expectedBindings: [{ name: "ASSETS", type: "assets" }], + expectedMainModule: "index.js", + expectedCompatibilityDate: "2024-09-27", + expectedCompatibilityFlags: ["nodejs_compat"], + }); + await runWrangler("deploy"); + }); + it("should be able to upload an asset-only project", async () => { const assets = [ { filePath: "file-1.txt", content: "Content of file-1" }, diff --git a/packages/wrangler/src/assets.ts b/packages/wrangler/src/assets.ts index 2359b763123e..3e6a943a6157 100644 --- a/packages/wrangler/src/assets.ts +++ b/packages/wrangler/src/assets.ts @@ -347,6 +347,7 @@ export function processAssetsArg( const assetConfig = { html_handling: config.assets?.html_handling, not_found_handling: config.assets?.not_found_handling, + serve_directly: config.assets?.serve_directly, }; return { diff --git a/packages/wrangler/src/config/environment.ts b/packages/wrangler/src/config/environment.ts index 8e60e8f70b50..b7e95d0ec4de 100644 --- a/packages/wrangler/src/config/environment.ts +++ b/packages/wrangler/src/config/environment.ts @@ -948,6 +948,7 @@ export type Assets = { | "drop-trailing-slash" | "none"; not_found_handling?: "single-page-application" | "404-page" | "none"; + serve_directly?: boolean; }; export interface Observability { diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index 487ac4a2d39e..45ef783a13b6 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/packages/wrangler/src/config/validation.ts @@ -2168,12 +2168,22 @@ const validateAssetsConfig: ValidatorFn = (diagnostics, field, value) => { ["single-page-application", "404-page", "none"] ) && isValid; + isValid = + validateOptionalProperty( + diagnostics, + field, + "serve_directly", + (value as Assets).serve_directly, + "boolean" + ) && isValid; + isValid = validateAdditionalProperties(diagnostics, field, Object.keys(value), [ "directory", "binding", "html_handling", "not_found_handling", + "serve_directly", ]) && isValid; return isValid; diff --git a/packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts b/packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts index e51d5dd3eb39..82267ad2bd18 100644 --- a/packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts +++ b/packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts @@ -194,6 +194,7 @@ export function createWorkerUploadForm(worker: CfWorkerInit): FormData { const assetConfig = { html_handling: assets?.assetConfig?.html_handling, not_found_handling: assets?.assetConfig?.not_found_handling, + serve_directly: assets?.assetConfig?.serve_directly, }; // short circuit if static assets upload only