Skip to content

Commit

Permalink
fix: pass routes to dev session
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
threepointone committed Jun 9, 2022
1 parent b44cc26 commit 9d9d550
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 3 deletions.
7 changes: 7 additions & 0 deletions .changeset/curly-bulldogs-shake.md
Original file line number Diff line number Diff line change
@@ -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.
41 changes: 41 additions & 0 deletions packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,27 @@ describe("wrangler dev", () => {
});
});

describe("routes", () => {
it("should pass routes to <Dev/>", 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({
Expand All @@ -156,6 +177,7 @@ describe("wrangler dev", () => {
expect.objectContaining({
host: "some-host.com",
zone: "some-zone-id",
routes: undefined,
})
);
});
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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" },
],
})
);
});
Expand All @@ -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",
},
],
})
);
});
Expand All @@ -290,6 +325,7 @@ describe("wrangler dev", () => {
expect.objectContaining({
host: "111.222.333.some-host.com",
zone: "some-zone-id",
routes: undefined,
})
);
});
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
16 changes: 15 additions & 1 deletion packages/wrangler/src/create-worker-preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions packages/wrangler/src/dev/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -194,6 +196,7 @@ function DevSession(props: DevSessionProps) {
legacyEnv={props.legacyEnv}
zone={props.zone}
host={props.host}
routes={props.routes}
/>
);
}
Expand Down
6 changes: 6 additions & 0 deletions packages/wrangler/src/dev/remote.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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");
Expand All @@ -52,6 +54,7 @@ export function Remote(props: {
legacyEnv: props.legacyEnv,
zone: props.zone,
host: props.host,
routes: props.routes,
});

usePreviewServer({
Expand Down Expand Up @@ -87,6 +90,7 @@ export function useWorker(props: {
legacyEnv: boolean | undefined;
zone: string | undefined;
host: string | undefined;
routes: Route[] | undefined;
}): CfPreviewToken | undefined {
const {
name,
Expand Down Expand Up @@ -184,6 +188,7 @@ export function useWorker(props: {
legacyEnv: props.legacyEnv,
zone: props.zone,
host: props.host,
routes: props.routes,
},
abortController.signal
)
Expand Down Expand Up @@ -231,6 +236,7 @@ export function useWorker(props: {
props.legacyEnv,
props.zone,
props.host,
props.routes,
]);
return token;
}
9 changes: 7 additions & 2 deletions packages/wrangler/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
3 changes: 3 additions & 0 deletions packages/wrangler/src/worker.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { Route } from "./config/environment";

/**
* A Cloudflare account.
*/
Expand Down Expand Up @@ -178,4 +180,5 @@ export interface CfWorkerContext {
legacyEnv: boolean | undefined;
zone: string | undefined;
host: string | undefined;
routes: Route[] | undefined;
}

0 comments on commit 9d9d550

Please sign in to comment.