Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify router-worker logic to allow for running user-worker ahead of assets #7303

Merged
merged 1 commit into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/funny-dingos-guess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@cloudflare/workers-shared": minor
---

Option to invoke user worker ahead of assets
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { defineProject, mergeConfig } from "vitest/config";
import configShared from "../../vitest.shared";
import configShared from "../../../vitest.shared.js";

export default mergeConfig(
configShared,
defineProject({
test: {
include: ["asset-worker/tests/**.{test,spec}.{ts,js}"],
include: ["tests/**.{test,spec}.{ts,js}"],
globals: true,
},
})
Expand Down
1 change: 1 addition & 0 deletions packages/workers-shared/asset-worker/wrangler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ account_id = "0f1b8aa119a907021f659042f95ea9ba"
workers_dev = false
main = "src/index.ts"
compatibility_date = "2024-07-31"
# nodejs_compat required when using @cloudflare/vitest-pool-workers
compatibility_flags = ["nodejs_compat"]

[[unsafe.bindings]]
Expand Down
9 changes: 6 additions & 3 deletions packages/workers-shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,16 @@
"bundle:router-worker:prod": "pnpm run bundle:router-worker --minify",
"check:lint": "eslint . --max-warnings=0",
"check:type": "pnpm run check:type:tests && tsc",
"check:type:tests": "tsc -p ./asset-worker/tests/tsconfig.json",
"check:type:tests": "tsc -p ./asset-worker/tests/tsconfig.json && tsc -p ./router-worker/tests/tsconfig.json",
"clean": "rimraf dist",
"deploy": "pnpm run deploy:router-worker && pnpm run deploy:asset-worker",
"deploy:asset-worker": "CLOUDFLARE_API_TOKEN=$WORKERS_DEPLOY_AND_CONFIG_CLOUDFLARE_API_TOKEN pnpx wrangler versions upload --experimental-versions -c asset-worker/wrangler.toml",
"deploy:router-worker": "CLOUDFLARE_API_TOKEN=$WORKERS_DEPLOY_AND_CONFIG_CLOUDFLARE_API_TOKEN pnpx wrangler versions upload --experimental-versions -c router-worker/wrangler.toml",
"dev": "pnpm run clean && concurrently -n bundle:asset-worker,bundle:router-worker -c blue,magenta \"pnpm run bundle:asset-worker --watch\" \"pnpm run bundle:router-worker --watch\"",
"test": "vitest",
"test:ci": "vitest run",
"test": "concurrently --group -n router-worker,asset-worker \"pnpm run test:router-worker\" \"pnpm run test:asset-worker\"",
"test:asset-worker": "vitest -c asset-worker/vitest.config.mts --dir asset-worker",
"test:ci": "pnpm run test",
"test:router-worker": "vitest -c router-worker/vitest.config.mts --dir router-worker",
"types:emit": "tsc index.ts --declaration --emitDeclarationOnly --declarationDir ./dist"
},
"dependencies": {
Expand All @@ -46,6 +48,7 @@
},
"devDependencies": {
"@cloudflare/eslint-config-worker": "workspace:*",
"@cloudflare/vitest-pool-workers": "latest",
WillTaylorDev marked this conversation as resolved.
Show resolved Hide resolved
"@cloudflare/workers-tsconfig": "workspace:*",
"@cloudflare/workers-types": "^4.20241106.0",
"@types/mime": "^3.0.4",
Expand Down
13 changes: 13 additions & 0 deletions packages/workers-shared/router-worker/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ export default {
}

const maybeSecondRequest = request.clone();

// User's configuration indicates they want user-Worker to run ahead of any
// assets. Do not provide any fallback logic.
if (env.CONFIG.invoke_user_worker_ahead_of_assets) {
if (!env.CONFIG.has_user_worker) {
throw new Error(
"Fetch for user worker without having a user worker binding"
);
}
return env.USER_WORKER.fetch(maybeSecondRequest);
}

// Otherwise, we try to first fetch assets, falling back to user-Worker.
if (env.CONFIG.has_user_worker) {
if (await env.ASSET_WORKER.unstable_canFetch(request)) {
analytics.setData({ dispatchtype: DISPATCH_TYPE.ASSETS });
Expand Down
74 changes: 74 additions & 0 deletions packages/workers-shared/router-worker/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { createExecutionContext } from "cloudflare:test";
import { describe, expect, it } from "vitest";
import { default as worker } from "../src/index";

describe("unit tests", async () => {
it("fails if specify running user worker ahead of assets, without user worker", async () => {
const request = new Request("https://example.com");
const ctx = createExecutionContext();

const env = {
CONFIG: {
invoke_user_worker_ahead_of_assets: true,
has_user_worker: false,
},
} as typeof env;

void expect(
async () => await worker.fetch(request, env, ctx)
).rejects.toThrowError(
"Fetch for user worker without having a user worker binding"
);
});

it("it returns fetch from user worker when invoke_user_worker_ahead_of_assets true", async () => {
const request = new Request("https://example.com");
const ctx = createExecutionContext();

const env = {
CONFIG: {
invoke_user_worker_ahead_of_assets: true,
has_user_worker: true,
},
USER_WORKER: {
async fetch(_: Request): Promise<Response> {
return new Response("hello from user worker");
},
},
WillTaylorDev marked this conversation as resolved.
Show resolved Hide resolved
ASSET_WORKER: {
async fetch(_: Request): Promise<Response> {
return new Response("hello from asset worker");
},
async unstable_canFetch(_: Request): Promise<boolean> {
return true;
},
},
} as typeof env;

const response = await worker.fetch(request, env, ctx);
expect(await response.text()).toEqual("hello from user worker");
});

it("it returns fetch from asset worker when matching existing asset path", async () => {
const request = new Request("https://example.com");
const ctx = createExecutionContext();

const env = {
CONFIG: {
invoke_user_worker_ahead_of_assets: false,
has_user_worker: false,
},
ASSET_WORKER: {
async fetch(_: Request): Promise<Response> {
return new Response("hello from asset worker");
},
async unstable_canFetch(_: Request): Promise<boolean> {
return true;
},
},
} as typeof env;

const response = await worker.fetch(request, env, ctx);
expect(await response.text()).toEqual("hello from asset worker");
});
});
15 changes: 15 additions & 0 deletions packages/workers-shared/router-worker/tests/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"compilerOptions": {
"target": "ES2020",
"module": "ES2020",
"lib": ["ES2020"],
"types": [
"@cloudflare/workers-types/experimental",
"@cloudflare/vitest-pool-workers"
],
"moduleResolution": "bundler",
"noEmit": true,
"skipLibCheck": true
},
"include": ["**/*.ts"]
}
13 changes: 13 additions & 0 deletions packages/workers-shared/router-worker/vitest.config.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { defineWorkersConfig } from "@cloudflare/vitest-pool-workers/config";

export default defineWorkersConfig({
test: {
poolOptions: {
workers: {
wrangler: {
configPath: "./wrangler.toml",
},
},
},
},
});
2 changes: 2 additions & 0 deletions packages/workers-shared/router-worker/wrangler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ account_id = "0f1b8aa119a907021f659042f95ea9ba"
workers_dev = false
main = "src/index.ts"
compatibility_date = "2024-07-31"
# nodejs_compat required when using @cloudflare/vitest-pool-workers
compatibility_flags = ["nodejs_compat", "no_nodejs_compat_v2"]

[version_metadata]
binding = "VERSION_METADATA"
Expand Down
2 changes: 1 addition & 1 deletion packages/workers-shared/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"sourceMap": true,
"forceConsistentCasingInFileNames": true,
"useUnknownInCatchVariables": false,
"types": ["@cloudflare/workers-types/experimental"]
"types": ["@cloudflare/workers-types/experimental", "@types/node"]
},
"include": ["**/*.ts", "vitest.config.mts"],
"exclude": ["node_modules", "dist", "**/tests", "**/*.test.ts"]
Expand Down
1 change: 1 addition & 0 deletions packages/workers-shared/utils/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { z } from "zod";

export const RoutingConfigSchema = z.object({
has_user_worker: z.boolean().optional(),
invoke_user_worker_ahead_of_assets: z.boolean().optional(),
});

export const AssetConfigSchema = z.object({
Expand Down
109 changes: 109 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading