From ea4db572c8feecba45b9194f38753ebbe9cb1944 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 14 Jan 2022 12:43:27 +0000 Subject: [PATCH] Refactor `pages` code to pass strict-null checks --- .changeset/big-steaks-design.md | 5 +++ .../pages/functions/filepath-routing.ts | 21 ++++++---- packages/wrangler/pages/functions/routes.ts | 2 +- .../pages/functions/template-worker.ts | 40 ++++++++----------- packages/wrangler/src/pages.tsx | 25 ++++++++---- 5 files changed, 52 insertions(+), 41 deletions(-) create mode 100644 .changeset/big-steaks-design.md diff --git a/.changeset/big-steaks-design.md b/.changeset/big-steaks-design.md new file mode 100644 index 000000000000..461d5f96a63e --- /dev/null +++ b/.changeset/big-steaks-design.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +Refactor pages code to pass strict-null checks diff --git a/packages/wrangler/pages/functions/filepath-routing.ts b/packages/wrangler/pages/functions/filepath-routing.ts index 401893fc5a97..dd37b1289678 100644 --- a/packages/wrangler/pages/functions/filepath-routing.ts +++ b/packages/wrangler/pages/functions/filepath-routing.ts @@ -52,7 +52,7 @@ export async function generateConfigFromFileTree({ const declaration = node.declaration; // `export async function onRequest() {...}` - if (declaration.type === "FunctionDeclaration") { + if (declaration.type === "FunctionDeclaration" && declaration.id) { exportNames.push(declaration.id.name); } @@ -155,12 +155,10 @@ export async function generateConfigFromFileTree({ // more specific routes aren't occluded from matching due to // less specific routes appearing first in the route list. export function compareRoutes(a: string, b: string) { - function parseRoutePath(routePath: string) { - let [method, segmentedPath] = routePath.split(" "); - if (!segmentedPath) { - segmentedPath = method; - method = null; - } + function parseRoutePath(routePath: string): [string | null, string[]] { + const parts = routePath.split(" ", 2); + const segmentedPath = parts.pop(); + const method = parts.pop() ?? null; const segments = segmentedPath.slice(1).split("/").filter(Boolean); return [method, segments]; @@ -204,7 +202,7 @@ async function forEachFile( const searchPaths = [baseDir]; const returnValues: T[] = []; - while (searchPaths.length) { + while (isNotEmpty(searchPaths)) { const cwd = searchPaths.shift(); const dir = await fs.readdir(cwd, { withFileTypes: true }); for (const entry of dir) { @@ -219,3 +217,10 @@ async function forEachFile( return returnValues; } + +interface NonEmptyArray extends Array { + shift(): T; +} +function isNotEmpty(array: T[]): array is NonEmptyArray { + return array.length > 0; +} diff --git a/packages/wrangler/pages/functions/routes.ts b/packages/wrangler/pages/functions/routes.ts index 4f535a89c16f..e10c9fad735b 100755 --- a/packages/wrangler/pages/functions/routes.ts +++ b/packages/wrangler/pages/functions/routes.ts @@ -114,7 +114,7 @@ export function parseConfig(config: Config, baseDir: string) { }); } - for (const [route, props] of Object.entries(config.routes)) { + for (const [route, props] of Object.entries(config.routes ?? {})) { let [_methods, routePath] = route.split(" "); if (!routePath) { routePath = _methods; diff --git a/packages/wrangler/pages/functions/template-worker.ts b/packages/wrangler/pages/functions/template-worker.ts index c462eb16da34..bebdc7b97603 100644 --- a/packages/wrangler/pages/functions/template-worker.ts +++ b/packages/wrangler/pages/functions/template-worker.ts @@ -33,17 +33,16 @@ declare const routes: RouteHandler[]; declare const __FALLBACK_SERVICE__: string; // expect an ASSETS fetcher binding pointing to the asset-server stage -type Env = { - [name: string]: unknown; - ASSETS: { fetch(url: string, init: RequestInit): Promise }; +type FetchEnv = { + [name: string]: { fetch: typeof fetch }; + ASSETS: { fetch: typeof fetch }; }; type WorkerContext = { waitUntil: (promise: Promise) => void; }; -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- `env` can be used by __FALLBACK_SERVICE_FETCH__ -function* executeRequest(request: Request, env: Env) { +function* executeRequest(request: Request, _env: FetchEnv) { const requestPath = new URL(request.url).pathname; // First, iterate through the routes (backwards) and execute "middlewares" on partial route matches @@ -88,20 +87,10 @@ function* executeRequest(request: Request, env: Env) { break; } } - - // Finally, yield to the fallback service (`env.ASSETS.fetch` in Pages' case) - return { - handler: () => - __FALLBACK_SERVICE__ - ? // @ts-expect-error expecting __FALLBACK_SERVICE__ to be the name of a service binding, so fetch should be defined - env[__FALLBACK_SERVICE__].fetch(request) - : fetch(request), - params: {} as Params, - }; } export default { - async fetch(request: Request, env: Env, workerContext: WorkerContext) { + async fetch(request: Request, env: FetchEnv, workerContext: WorkerContext) { const handlerIterator = executeRequest(request, env); const data = {}; // arbitrary data the user can set between functions const next = async (input?: RequestInfo, init?: RequestInit) => { @@ -109,14 +98,11 @@ export default { request = new Request(input, init); } - const { value } = handlerIterator.next(); - if (value) { - const { handler, params } = value; - const context: EventContext< - unknown, - string, - Record - > = { + const result = handlerIterator.next(); + // Note we can't use `!result.done` because this doesn't narrow to the correct type + if (result.done == false) { + const { handler, params } = result.value; + const context = { request: new Request(request.clone()), next, params, @@ -132,6 +118,12 @@ export default { [101, 204, 205, 304].includes(response.status) ? null : response.body, response ); + } else if (__FALLBACK_SERVICE__) { + // There are no more handlers so finish with the fallback service (`env.ASSETS.fetch` in Pages' case) + return env[__FALLBACK_SERVICE__].fetch(request); + } else { + // There was not fallback service so actually make the request to the origin. + return fetch(request); } }; diff --git a/packages/wrangler/src/pages.tsx b/packages/wrangler/src/pages.tsx index 55aecf50d97c..033bbc19f81b 100644 --- a/packages/wrangler/src/pages.tsx +++ b/packages/wrangler/src/pages.tsx @@ -1,6 +1,5 @@ /* eslint-disable no-shadow */ -import assert from "assert"; import type { BuilderCallback } from "yargs"; import { join } from "path"; import { tmpdir } from "os"; @@ -22,7 +21,7 @@ import { generateConfigFromFileTree } from "../pages/functions/filepath-routing" import type { Headers, Request, fetch } from "@miniflare/core"; import type { MiniflareOptions } from "miniflare"; -const EXIT_CALLBACKS = []; +const EXIT_CALLBACKS: (() => void)[] = []; const EXIT = (message?: string, code?: number) => { if (message) console.log(message); if (code) process.exitCode = code; @@ -122,7 +121,9 @@ async function spawnProxyProcess({ }, } ); - EXIT_CALLBACKS.push(() => proxy.kill()); + EXIT_CALLBACKS.push(() => { + proxy.kill(); + }); proxy.stdout.on("data", (data) => { console.log(`[proxy]: ${data}`); @@ -862,11 +863,13 @@ export const pages: BuilderCallback = (yargs) => { // internally just waits for that promise to resolve. await scriptReadyPromise; - // Should only be called if no proxyPort, using `assert.fail()` here - // means the type of `assetsFetch` is still `typeof fetch` - const assetsFetch = proxyPort - ? () => assert.fail() - : await generateAssetsFetch(directory); + // `assetsFetch()` will only be called if there is `proxyPort` defined. + // We only define `proxyPort`, above, when there is no `directory` defined. + const assetsFetch = + directory !== undefined + ? await generateAssetsFetch(directory) + : invalidAssetsFetch; + const miniflare = new Miniflare({ port, watch: true, @@ -1029,3 +1032,9 @@ export const pages: BuilderCallback = (yargs) => { ) ); }; + +const invalidAssetsFetch: typeof fetch = () => { + throw new Error( + "Trying to fetch assets directly when there is no `directory` option specified, and not in `local` mode." + ); +};