Skip to content

Commit 00cf67c

Browse files
GregBrimbleGregBrimble
authored
Revert "Upload metafiles for Workers Assets (#8255)" (#8336)
This reverts commit 38c298f. This PR was triggereing C3 e2e test failure for the React+Vite experimental template.
1 parent 94729a6 commit 00cf67c

File tree

29 files changed

+240
-382
lines changed

29 files changed

+240
-382
lines changed

‎.changeset/chatty-seas-grin.md‎

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎fixtures/asset-config/html-handling.test.ts‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,15 @@ describe.each(testSuites)("$title", ({ title, suite }) => {
5959
vi.mocked(getAssetWithMetadataFromKV).mockRestore();
6060
});
6161
describe.each(suite)(`$html_handling`, ({ html_handling, cases }) => {
62-
beforeEach(async () => {
63-
const originalApplyConfigurationDefaults = (
64-
await vi.importActual<
65-
typeof import("../../packages/workers-shared/asset-worker/src/configuration")
66-
>("../../packages/workers-shared/asset-worker/src/configuration")
67-
).applyConfigurationDefaults;
68-
vi.mocked(applyConfigurationDefaults).mockImplementation(() => ({
69-
...originalApplyConfigurationDefaults({}),
70-
html_handling,
71-
not_found_handling: "none",
72-
}));
62+
beforeEach(() => {
63+
vi.mocked(applyConfigurationDefaults).mockImplementation(() => {
64+
return {
65+
html_handling,
66+
not_found_handling: "none",
67+
run_worker_first: true,
68+
serve_directly: false,
69+
};
70+
});
7371
});
7472
it.each(cases)(
7573
"$title",

‎fixtures/asset-config/redirects.test.ts‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ describe("[Asset Worker] `test location rewrite`", () => {
2424
afterEach(() => {
2525
vi.mocked(getAssetWithMetadataFromKV).mockRestore();
2626
});
27-
beforeEach(async () => {
27+
beforeEach(() => {
2828
vi.mocked(getAssetWithMetadataFromKV).mockImplementation(
2929
() =>
3030
Promise.resolve({
@@ -37,16 +37,14 @@ describe("[Asset Worker] `test location rewrite`", () => {
3737
>
3838
);
3939

40-
const originalApplyConfigurationDefaults = (
41-
await vi.importActual<
42-
typeof import("../../packages/workers-shared/asset-worker/src/configuration")
43-
>("../../packages/workers-shared/asset-worker/src/configuration")
44-
).applyConfigurationDefaults;
45-
vi.mocked(applyConfigurationDefaults).mockImplementation(() => ({
46-
...originalApplyConfigurationDefaults({}),
47-
html_handling: "none",
48-
not_found_handling: "none",
49-
}));
40+
vi.mocked(applyConfigurationDefaults).mockImplementation(() => {
41+
return {
42+
html_handling: "none",
43+
not_found_handling: "none",
44+
run_worker_first: true,
45+
serve_directly: false,
46+
};
47+
});
5048
});
5149

5250
it("returns 404 for non matched encoded url", async () => {

‎fixtures/asset-config/url-normalization.test.ts‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { SELF } from "cloudflare:test";
2-
import { afterEach, beforeEach, describe, expect, it, Mock, vi } from "vitest";
2+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
33
import { applyConfigurationDefaults } from "../../packages/workers-shared/asset-worker/src/configuration";
44
import Worker from "../../packages/workers-shared/asset-worker/src/index";
55
import { getAssetWithMetadataFromKV } from "../../packages/workers-shared/asset-worker/src/utils/kv";
@@ -24,7 +24,7 @@ describe("[Asset Worker] `test slash normalization`", () => {
2424
afterEach(() => {
2525
vi.mocked(getAssetWithMetadataFromKV).mockRestore();
2626
});
27-
beforeEach(async () => {
27+
beforeEach(() => {
2828
vi.mocked(getAssetWithMetadataFromKV).mockImplementation(
2929
() =>
3030
Promise.resolve({
@@ -37,16 +37,14 @@ describe("[Asset Worker] `test slash normalization`", () => {
3737
>
3838
);
3939

40-
const originalApplyConfigurationDefaults = (
41-
await vi.importActual<
42-
typeof import("../../packages/workers-shared/asset-worker/src/configuration")
43-
>("../../packages/workers-shared/asset-worker/src/configuration")
44-
).applyConfigurationDefaults;
45-
vi.mocked(applyConfigurationDefaults).mockImplementation(() => ({
46-
...originalApplyConfigurationDefaults({}),
47-
html_handling: "none",
48-
not_found_handling: "none",
49-
}));
40+
vi.mocked(applyConfigurationDefaults).mockImplementation(() => {
41+
return {
42+
html_handling: "none",
43+
not_found_handling: "none",
44+
run_worker_first: true,
45+
serve_directly: false,
46+
};
47+
});
5048
});
5149

5250
it("returns 200 leading encoded double slash", async () => {

‎packages/miniflare/src/plugins/assets/index.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,7 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
9999
const assetService: Service = {
100100
name: `${ASSETS_SERVICE_NAME}:${id}`,
101101
worker: {
102-
// TODO: read these from the wrangler.toml
103-
compatibilityDate: "2024-07-31",
104-
compatibilityFlags: ["nodejs_compat"],
102+
compatibilityDate: "2024-08-01",
105103
modules: [
106104
{
107105
name: "asset-worker.mjs",
@@ -130,9 +128,7 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
130128
const routerService: Service = {
131129
name: `${ROUTER_SERVICE_NAME}:${id}`,
132130
worker: {
133-
// TODO: read these from the wrangler.toml
134-
compatibilityDate: "2024-07-31",
135-
compatibilityFlags: ["nodejs_compat", "no_nodejs_compat_v2"],
131+
compatibilityDate: "2024-08-01",
136132
modules: [
137133
{
138134
name: "router-worker.mjs",
@@ -152,7 +148,7 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
152148
},
153149
{
154150
name: "CONFIG",
155-
json: JSON.stringify(options.assets.routerConfig),
151+
json: JSON.stringify(options.assets.routingConfig),
156152
},
157153
],
158154
},

‎packages/miniflare/src/plugins/assets/schema.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {
22
AssetConfigSchema,
3-
RouterConfigSchema,
3+
RoutingConfigSchema,
44
} from "@cloudflare/workers-shared";
55
import { z } from "zod";
66
import { PathSchema } from "../../shared";
@@ -13,7 +13,7 @@ export const AssetsOptionsSchema = z.object({
1313
workerName: z.string().optional(),
1414
directory: PathSchema,
1515
binding: z.string().optional(),
16-
routerConfig: RouterConfigSchema.optional(),
16+
routingConfig: RoutingConfigSchema.optional(),
1717
assetConfig: AssetConfigSchema.optional(),
1818
})
1919
.optional(),

‎packages/vitest-pool-workers/src/pool/config.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,8 @@ async function parseCustomPoolOptions(
218218
if (options.miniflare?.assets) {
219219
// (Used to set the SELF binding to point to the router worker instead)
220220
options.miniflare.hasAssetsAndIsVitest = true;
221-
options.miniflare.assets.routerConfig ??= {};
222-
options.miniflare.assets.routerConfig.has_user_worker = Boolean(
221+
options.miniflare.assets.routingConfig ??= {};
222+
options.miniflare.assets.routingConfig.has_user_worker = Boolean(
223223
options.main
224224
);
225225
}

‎packages/workers-shared/asset-worker/src/configuration.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,19 @@ import type { AssetConfig } from "../../utils/types";
33
export const applyConfigurationDefaults = (
44
configuration?: AssetConfig
55
): Required<AssetConfig> => {
6+
let runWorkerFirst = undefined;
7+
if (configuration?.run_worker_first !== undefined) {
8+
runWorkerFirst = configuration?.run_worker_first;
9+
} else if (configuration?.serve_directly !== undefined) {
10+
runWorkerFirst = !configuration.serve_directly;
11+
} else {
12+
runWorkerFirst = false;
13+
}
14+
615
return {
7-
compatibility_date: configuration?.compatibility_date ?? "2021-11-02",
8-
compatibility_flags: configuration?.compatibility_flags ?? [],
916
html_handling: configuration?.html_handling ?? "auto-trailing-slash",
1017
not_found_handling: configuration?.not_found_handling ?? "none",
11-
account_id: configuration?.account_id ?? -1,
12-
script_id: configuration?.script_id ?? -1,
18+
run_worker_first: runWorkerFirst,
19+
serve_directly: !runWorkerFirst,
1320
};
1421
};

‎packages/workers-shared/asset-worker/src/handler.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ export const handleRequest = async (
3838
});
3939
}
4040

41+
// if there was a POST etc. to a route without an asset
42+
// this should be passed onto a user worker if one exists
43+
// so prioritise returning a 404 over 405?
4144
const method = request.method.toUpperCase();
4245
if (!["GET", "HEAD"].includes(method)) {
4346
return env.JAEGER.enterSpan("method_not_allowed", (span) => {
@@ -718,7 +721,7 @@ const safeRedirect = async (
718721
* +===========================================+===========+======================+
719722
* | unreserved ASCII e.g. a-z | unchanged | unchanged |
720723
* +-------------------------------------------+-----------+----------------------+
721-
* | reserved (sometimes encoded) | unchanged | encoded |
724+
* | reserved (sometimes encoded) | unchanged | encoded |
722725
* | e.g. [ ] @ $ ! ' ( ) * + , ; = : ? # & % | | |
723726
* +-------------------------------------------+-----------+----------------------+
724727
* | non-ASCII e.g. ü. and space | encoded | encoded |

‎packages/workers-shared/asset-worker/src/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { ExperimentAnalytics } from "./experiment-analytics";
1010
import { decodePath, getIntent, handleRequest } from "./handler";
1111
import { getAssetWithMetadataFromKV } from "./utils/kv";
1212
import type {
13-
AssetConfig,
13+
AssetWorkerConfig,
1414
ColoMetadata,
1515
JaegerTracing,
1616
UnsafePerformanceTimer,
@@ -31,7 +31,7 @@ export type Env = {
3131
*/
3232
ASSETS_KV_NAMESPACE: KVNamespace;
3333

34-
CONFIG: AssetConfig;
34+
CONFIG: AssetWorkerConfig;
3535

3636
SENTRY_DSN: string;
3737
SENTRY_ACCESS_CLIENT_ID: string;

0 commit comments

Comments
 (0)