From 9d9d550634bff4463e969993b6ce99f627068675 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 9 Jun 2022 13:33:42 +0100 Subject: [PATCH] fix: pass `routes` to `dev` session We can pass routes when creating a `dev` session. The effect of this is when you visit a path that _doesn't_ match the given routes, then it instead does a fetch from the deployed worker on that path (if any). We were previously passing `*/*`, i.e, matching _all_ routes in dev; this fix now passes configured routes instead. --- .changeset/curly-bulldogs-shake.md | 7 ++++ packages/wrangler/src/__tests__/dev.test.tsx | 41 +++++++++++++++++++ .../wrangler/src/create-worker-preview.ts | 16 +++++++- packages/wrangler/src/dev/dev.tsx | 3 ++ packages/wrangler/src/dev/remote.tsx | 6 +++ packages/wrangler/src/index.tsx | 9 +++- packages/wrangler/src/worker.ts | 3 ++ 7 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 .changeset/curly-bulldogs-shake.md diff --git a/.changeset/curly-bulldogs-shake.md b/.changeset/curly-bulldogs-shake.md new file mode 100644 index 000000000000..da7e87cf2d0e --- /dev/null +++ b/.changeset/curly-bulldogs-shake.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +fix: pass `routes` to `dev` session + +We can pass routes when creating a `dev` session. The effect of this is when you visit a path that _doesn't_ match the given routes, then it instead does a fetch from the deployed worker on that path (if any). We were previously passing `*/*`, i.e, matching _all_ routes in dev; this fix now passes configured routes instead. diff --git a/packages/wrangler/src/__tests__/dev.test.tsx b/packages/wrangler/src/__tests__/dev.test.tsx index ecc6179147d2..82dfd90c866f 100644 --- a/packages/wrangler/src/__tests__/dev.test.tsx +++ b/packages/wrangler/src/__tests__/dev.test.tsx @@ -144,6 +144,27 @@ describe("wrangler dev", () => { }); }); + describe("routes", () => { + it("should pass routes to ", async () => { + fs.writeFileSync("index.js", `export default {};`); + + // config.routes + mockGetZones("5.some-host.com", [{ id: "some-zone-id-5" }]); + writeWranglerToml({ + main: "index.js", + routes: ["http://5.some-host.com/some/path/*"], + }); + await runWrangler("dev"); + expect((Dev as jest.Mock).mock.calls[0][0]).toEqual( + expect.objectContaining({ + host: "5.some-host.com", + zone: "some-zone-id-5", + routes: ["http://5.some-host.com/some/path/*"], + }) + ); + }); + }); + describe("host", () => { it("should resolve a host to its zone", async () => { writeWranglerToml({ @@ -156,6 +177,7 @@ describe("wrangler dev", () => { expect.objectContaining({ host: "some-host.com", zone: "some-zone-id", + routes: undefined, }) ); }); @@ -217,6 +239,10 @@ describe("wrangler dev", () => { mockGetZones("some-host.com", [{ id: "some-zone-id" }]); await runWrangler("dev --env staging"); expect((Dev as jest.Mock).mock.calls[0][0].host).toEqual("some-host.com"); + expect((Dev as jest.Mock).mock.calls[0][0].routes).toEqual([ + "http://some-host.com/some/path/*", + "http://some-other-host.com/path/*", + ]); }); it("should strip leading `*` from given host when deducing a zone id", async () => { @@ -254,6 +280,9 @@ describe("wrangler dev", () => { expect.objectContaining({ host: "some-domain.com", zone: "some-zone-id", + routes: [ + { pattern: "https://some-domain.com/*", zone_id: "some-zone-id" }, + ], }) ); }); @@ -273,6 +302,12 @@ describe("wrangler dev", () => { // note that it uses the provided zone_name as a host too host: "some-zone.com", zone: "a-zone-id", + routes: [ + { + pattern: "https://some-domain.com/*", + zone_name: "some-zone.com", + }, + ], }) ); }); @@ -290,6 +325,7 @@ describe("wrangler dev", () => { expect.objectContaining({ host: "111.222.333.some-host.com", zone: "some-zone-id", + routes: undefined, }) ); }); @@ -310,6 +346,7 @@ describe("wrangler dev", () => { expect.objectContaining({ host: "5.some-host.com", zone: "some-zone-id-5", + routes: ["http://5.some-host.com/some/path/*"], }) ); (Dev as jest.Mock).mockClear(); @@ -325,6 +362,7 @@ describe("wrangler dev", () => { expect.objectContaining({ host: "4.some-host.com", zone: "some-zone-id-4", + routes: ["https://4.some-host.com/some/path/*"], }) ); (Dev as jest.Mock).mockClear(); @@ -340,6 +378,7 @@ describe("wrangler dev", () => { expect.objectContaining({ host: "3.some-host.com", zone: "some-zone-id-3", + routes: ["http://3.some-host.com/some/path/*"], }) ); (Dev as jest.Mock).mockClear(); @@ -358,6 +397,7 @@ describe("wrangler dev", () => { expect.objectContaining({ host: "2.some-host.com", zone: "some-zone-id-2", + routes: ["http://3.some-host.com/some/path/*"], }) ); (Dev as jest.Mock).mockClear(); @@ -378,6 +418,7 @@ describe("wrangler dev", () => { expect.objectContaining({ host: "1.some-host.com", zone: "some-zone-id-1", + routes: ["http://3.some-host.com/some/path/*"], }) ); (Dev as jest.Mock).mockClear(); diff --git a/packages/wrangler/src/create-worker-preview.ts b/packages/wrangler/src/create-worker-preview.ts index ed0c0d3c9c51..210af817fd52 100644 --- a/packages/wrangler/src/create-worker-preview.ts +++ b/packages/wrangler/src/create-worker-preview.ts @@ -118,7 +118,21 @@ async function createPreviewToken( : `/accounts/${accountId}/workers/scripts/${scriptId}/edge-preview`; const mode: CfPreviewMode = ctx.zone - ? { routes: ["*/*"] } // TODO: should we support routes here? how? + ? { + routes: ctx.routes + ? // extract all the route patterns + ctx.routes.map((route) => { + if (typeof route === "string") { + return route; + } + if (route.custom_domain) { + return `${route.pattern}/*`; + } + return route.pattern; + }) + : // if there aren't any patterns, then just match on all routes + ["*/*"], + } : { workers_dev: true }; const formData = createWorkerUploadForm(worker); diff --git a/packages/wrangler/src/dev/dev.tsx b/packages/wrangler/src/dev/dev.tsx index e34a26499961..b07657f6fcaa 100644 --- a/packages/wrangler/src/dev/dev.tsx +++ b/packages/wrangler/src/dev/dev.tsx @@ -18,6 +18,7 @@ import { Local } from "./local"; import { Remote } from "./remote"; import { useEsbuild } from "./use-esbuild"; import type { Config } from "../config"; +import type { Route } from "../config/environment"; import type { Entry } from "../entry"; import type { AssetPaths } from "../sites"; import type { CfWorkerInit } from "../worker"; @@ -56,6 +57,7 @@ export type DevProps = { legacyEnv: boolean; zone: string | undefined; host: string | undefined; + routes: Route[] | undefined; }; export function DevImplementation(props: DevProps): JSX.Element { @@ -194,6 +196,7 @@ function DevSession(props: DevSessionProps) { legacyEnv={props.legacyEnv} zone={props.zone} host={props.host} + routes={props.routes} /> ); } diff --git a/packages/wrangler/src/dev/remote.tsx b/packages/wrangler/src/dev/remote.tsx index d65ad831c22d..39d2758935cc 100644 --- a/packages/wrangler/src/dev/remote.tsx +++ b/packages/wrangler/src/dev/remote.tsx @@ -7,6 +7,7 @@ import useInspector from "../inspect"; import { logger } from "../logger"; import { usePreviewServer } from "../proxy"; import { syncAssets } from "../sites"; +import type { Route } from "../config/environment"; import type { CfPreviewToken } from "../create-worker-preview"; import type { AssetPaths } from "../sites"; import type { CfModule, CfWorkerInit, CfScriptFormat } from "../worker"; @@ -32,6 +33,7 @@ export function Remote(props: { legacyEnv: boolean | undefined; zone: string | undefined; host: string | undefined; + routes: Route[] | undefined; }) { assert(props.accountId, "accountId is required"); assert(props.apiToken, "apiToken is required"); @@ -52,6 +54,7 @@ export function Remote(props: { legacyEnv: props.legacyEnv, zone: props.zone, host: props.host, + routes: props.routes, }); usePreviewServer({ @@ -87,6 +90,7 @@ export function useWorker(props: { legacyEnv: boolean | undefined; zone: string | undefined; host: string | undefined; + routes: Route[] | undefined; }): CfPreviewToken | undefined { const { name, @@ -184,6 +188,7 @@ export function useWorker(props: { legacyEnv: props.legacyEnv, zone: props.zone, host: props.host, + routes: props.routes, }, abortController.signal ) @@ -231,6 +236,7 @@ export function useWorker(props: { props.legacyEnv, props.zone, props.host, + props.routes, ]); return token; } diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index 51ab543acebe..7a11a187b9cd 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -69,6 +69,7 @@ import { whoami } from "./whoami"; import { getZoneIdFromHost, getZoneForRoute } from "./zones"; import type { Config } from "./config"; +import type { Route } from "./config/environment"; import type { TailCLIFilters } from "./tail"; import type { RawData } from "ws"; import type { CommandModule } from "yargs"; @@ -1090,14 +1091,16 @@ function createCLIParser(argv: string[]) { // Compute zone info from the `host` and `route` args and config; let host = args.host || config.dev.host; let zoneId: string | undefined; + const routes: Route[] | undefined = + args.routes || (config.route && [config.route]) || config.routes; if (!args.local) { if (host) { zoneId = await getZoneIdFromHost(host); } - const routes = args.routes || config.route || config.routes; + if (!zoneId && routes) { - const firstRoute = Array.isArray(routes) ? routes[0] : routes; + const firstRoute = routes[0]; const zone = await getZoneForRoute(firstRoute); if (zone) { zoneId = zone.id; @@ -1184,6 +1187,7 @@ function createCLIParser(argv: string[]) { env={args.env} zone={zoneId} host={host} + routes={routes} rules={getRules(config)} legacyEnv={isLegacyEnv(config)} minify={args.minify ?? config.minify} @@ -1613,6 +1617,7 @@ function createCLIParser(argv: string[]) { env={args.env} zone={undefined} host={undefined} + routes={undefined} legacyEnv={isLegacyEnv(config)} build={config.build || {}} minify={undefined} diff --git a/packages/wrangler/src/worker.ts b/packages/wrangler/src/worker.ts index 170a44fc11a0..a2a56b8a9513 100644 --- a/packages/wrangler/src/worker.ts +++ b/packages/wrangler/src/worker.ts @@ -1,3 +1,5 @@ +import type { Route } from "./config/environment"; + /** * A Cloudflare account. */ @@ -178,4 +180,5 @@ export interface CfWorkerContext { legacyEnv: boolean | undefined; zone: string | undefined; host: string | undefined; + routes: Route[] | undefined; }