From c7aa4ae4f7ee7152c0b792657482ab110422f2ee Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 11 Jan 2022 16:30:51 +0000 Subject: [PATCH] Enable TypeScript strictNullChecks across the codebase --- .vscode/settings.json | 14 +- .../pages/functions/filepath-routing.ts | 16 +- packages/wrangler/pages/functions/routes.ts | 2 +- .../pages/functions/template-worker.ts | 40 ++-- packages/wrangler/src/__tests__/kv.test.ts | 14 +- .../wrangler/src/__tests__/mock-cfetch.ts | 8 +- packages/wrangler/src/api/form_data.ts | 2 +- packages/wrangler/src/cfetch/internal.ts | 12 +- packages/wrangler/src/dev.tsx | 50 +++-- packages/wrangler/src/index.tsx | 197 +++++++++--------- packages/wrangler/src/inspect.ts | 52 +++-- packages/wrangler/src/kv.tsx | 27 ++- packages/wrangler/src/pages.tsx | 20 +- packages/wrangler/src/proxy.ts | 2 +- packages/wrangler/src/publish.ts | 49 +++-- packages/wrangler/src/sites.tsx | 8 +- packages/wrangler/src/user.tsx | 16 +- tsconfig.json | 3 +- 18 files changed, 287 insertions(+), 245 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 82a11dfbfb87..9147ed42a980 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -7,12 +7,22 @@ "esbuild", "estree", "execa", + "extensionless", "iarna", "keyvalue", "middlewares", "Miniflare", "outdir", "outfile", - "Positionals" - ] + "pgrep", + "PKCE", + "Positionals", + "undici", + "wasmvalue", + "weakmap", + "weakset", + "webassemblymemory", + "websockets" + ], + "cSpell.ignoreWords": ["yxxx"] } diff --git a/packages/wrangler/pages/functions/filepath-routing.ts b/packages/wrangler/pages/functions/filepath-routing.ts index 401893fc5a97..66f883938fb8 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]; @@ -205,7 +203,9 @@ async function forEachFile( const returnValues: T[] = []; while (searchPaths.length) { - const cwd = searchPaths.shift(); + // The `searchPaths.length` check above ensures that `searchPaths.shift()` is defined + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const cwd = searchPaths.shift()!; const dir = await fs.readdir(cwd, { withFileTypes: true }); for (const entry of dir) { const pathname = path.join(cwd, entry.name); 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..569f0a3f8e9b 100644 --- a/packages/wrangler/pages/functions/template-worker.ts +++ b/packages/wrangler/pages/functions/template-worker.ts @@ -110,29 +110,23 @@ export default { } const { value } = handlerIterator.next(); - if (value) { - const { handler, params } = value; - const context: EventContext< - unknown, - string, - Record - > = { - request: new Request(request.clone()), - next, - params, - data, - env, - waitUntil: workerContext.waitUntil.bind(workerContext), - }; - - const response = await handler(context); - - // https://fetch.spec.whatwg.org/#null-body-status - return new Response( - [101, 204, 205, 304].includes(response.status) ? null : response.body, - response - ); - } + const { handler, params } = value; + const context: EventContext> = { + request: new Request(request.clone()), + next, + params, + data, + env, + waitUntil: workerContext.waitUntil.bind(workerContext), + }; + + const response = await handler(context); + + // https://fetch.spec.whatwg.org/#null-body-status + return new Response( + [101, 204, 205, 304].includes(response.status) ? null : response.body, + response + ); }; try { diff --git a/packages/wrangler/src/__tests__/kv.test.ts b/packages/wrangler/src/__tests__/kv.test.ts index 88af107985f8..ad04f28ca50b 100644 --- a/packages/wrangler/src/__tests__/kv.test.ts +++ b/packages/wrangler/src/__tests__/kv.test.ts @@ -212,7 +212,7 @@ describe("wrangler", () => { it("should make multiple requests for paginated results", async () => { // Create a lot of mock namespaces, so that the fetch requests will be paginated - const KVNamespaces = []; + const KVNamespaces: { title: string; id: string }[] = []; for (let i = 0; i < 550; i++) { KVNamespaces.push({ title: "title-" + i, id: "id-" + i }); } @@ -335,8 +335,12 @@ describe("wrangler", () => { expect(namespaceId).toEqual(expectedNamespaceId); expect(key).toEqual(expectedKey); expect(body).toEqual(expectedValue); - expect(query.get("expiration")).toEqual(`${expiration}`); - expect(query.get("expiration_ttl")).toEqual(`${expirationTtl}`); + if (expiration) { + expect(query.get("expiration")).toEqual(`${expiration}`); + } + if (expirationTtl) { + expect(query.get("expiration_ttl")).toEqual(`${expirationTtl}`); + } return null; } ); @@ -681,7 +685,7 @@ describe("wrangler", () => { if (expectedKeys.length <= keysPerRequest) { return createFetchResult(expectedKeys); } else { - const start = parseInt(query.get("cursor")) || 0; + const start = parseInt(query.get("cursor") ?? "0") || 0; const end = start + keysPerRequest; const cursor = end < expectedKeys.length ? end : undefined; return createFetchResult( @@ -778,7 +782,7 @@ describe("wrangler", () => { it("should make multiple requests for paginated results", async () => { // Create a lot of mock keys, so that the fetch requests will be paginated - const keys = []; + const keys: string[] = []; for (let i = 0; i < 550; i++) { keys.push("key-" + i); } diff --git a/packages/wrangler/src/__tests__/mock-cfetch.ts b/packages/wrangler/src/__tests__/mock-cfetch.ts index f0ca6c035743..977308070313 100644 --- a/packages/wrangler/src/__tests__/mock-cfetch.ts +++ b/packages/wrangler/src/__tests__/mock-cfetch.ts @@ -1,5 +1,5 @@ import type { RequestInit } from "node-fetch"; -import type { URLSearchParams } from "node:url"; +import { URLSearchParams } from "node:url"; import { pathToRegexp } from "path-to-regexp"; import { CF_API_BASE_URL } from "../cfetch"; import type { FetchResult } from "../cfetch"; @@ -9,8 +9,8 @@ import type { FetchResult } from "../cfetch"; */ export type MockHandler = ( uri: RegExpExecArray, - init?: RequestInit, - queryParams?: URLSearchParams + init: RequestInit, + queryParams: URLSearchParams ) => ResponseType; type RemoveMockFn = () => void; @@ -32,7 +32,7 @@ const mocks: MockFetch[] = []; export async function mockFetchInternal( resource: string, init: RequestInit = {}, - queryParams?: URLSearchParams + queryParams: URLSearchParams = new URLSearchParams() ) { for (const { regexp, method, handler } of mocks) { const resourcePath = new URL(resource, CF_API_BASE_URL).pathname; diff --git a/packages/wrangler/src/api/form_data.ts b/packages/wrangler/src/api/form_data.ts index 90a3fb4fad9b..51eebb282db6 100644 --- a/packages/wrangler/src/api/form_data.ts +++ b/packages/wrangler/src/api/form_data.ts @@ -23,7 +23,7 @@ export function toMimeType(type: CfModuleType): string { } } -function toModule(module: CfModule, entryType?: CfModuleType): Blob { +function toModule(module: CfModule, entryType: CfModuleType): Blob { const { type: moduleType, content } = module; const type = toMimeType(moduleType ?? entryType); diff --git a/packages/wrangler/src/cfetch/internal.ts b/packages/wrangler/src/cfetch/internal.ts index d77163e404c9..f7bdeae7d349 100644 --- a/packages/wrangler/src/cfetch/internal.ts +++ b/packages/wrangler/src/cfetch/internal.ts @@ -1,5 +1,5 @@ -import fetch from "node-fetch"; -import type { RequestInit, HeadersInit } from "node-fetch"; +import fetch, { Headers } from "node-fetch"; +import type { RequestInit } from "node-fetch"; import { getAPIToken, loginOrRefreshIfRequired } from "../user"; export const CF_API_BASE_URL = @@ -21,7 +21,7 @@ export async function fetchInternal( ): Promise { await requireLoggedIn(); const apiToken = requireApiToken(); - const headers = cloneHeaders(init.headers); + const headers = new Headers(init.headers); addAuthorizationHeader(headers, apiToken); const queryString = queryParams ? `?${queryParams.toString()}` : ""; @@ -55,11 +55,7 @@ function requireApiToken(): string { return apiToken; } -function cloneHeaders(headers: HeadersInit): HeadersInit { - return { ...headers }; -} - -function addAuthorizationHeader(headers: HeadersInit, apiToken: string): void { +function addAuthorizationHeader(headers: Headers, apiToken: string): void { if (headers["Authorization"]) { throw new Error( "The request already specifies an authorisation header - cannot add a new one." diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index bcf46bb7ce1c..9dea6e36b7cc 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -56,7 +56,7 @@ function Dev(props: DevProps): JSX.Element { "You cannot use the service worker format with a `public` directory." ); } - const port = props.port || 8787; + const port = props.port ?? 8787; const apiToken = getAPIToken(); const directory = useTmpDir(); @@ -98,7 +98,7 @@ function Dev(props: DevProps): JSX.Element { bindings={props.bindings} site={props.site} public={props.public} - port={props.port} + port={port} /> ) : ( { + local.current.stdout?.on("data", (data: Buffer) => { console.log(`${data.toString()}`); }); - local.current.stderr.on("data", (data: Buffer) => { + local.current.stderr?.on("data", (data: Buffer) => { console.error(`${data.toString()}`); const matches = /Debugger listening on (ws:\/\/127\.0\.0\.1:9229\/[A-Za-z0-9-]+)/.exec( @@ -357,7 +357,7 @@ function useCustomBuild( ): undefined | string { const [entry, setEntry] = useState( // if there's no build command, just return the expected entry - props.command ? null : expectedEntry + props.command ? undefined : expectedEntry ); const { command, cwd, watch_dir } = props; useEffect(() => { @@ -461,25 +461,35 @@ function useEsbuild(props: { else { // nothing really changes here, so let's increment the id // to change the return object's identity - setBundle((previousBundle) => ({ - ...previousBundle, - id: previousBundle.id + 1, - })); + setBundle((previousBundle) => { + if (previousBundle === undefined) { + throw new Error( + "Rebuild triggered with no previous build available" + ); + } + return { ...previousBundle, id: previousBundle.id + 1 }; + }); } }, }, }); - const chunks = Object.entries(result.metafile.outputs).find( + // result.metafile is defined because of the `metafile: true` option above. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const outputEntry = Object.entries(result.metafile!.outputs).find( ([_path, { entryPoint }]) => entryPoint === entry ); // assumedly only one entry point - + if (outputEntry === undefined) { + throw new Error( + `Cannot find entry-point "${entry}" in generated bundle.` + ); + } setBundle({ id: 0, entry, - path: chunks[0], - type: chunks[1].exports.length > 0 ? "esm" : "commonjs", - exports: chunks[1].exports, + path: outputEntry[0], + type: outputEntry[1].exports.length > 0 ? "esm" : "commonjs", + exports: outputEntry[1].exports, modules: moduleCollector.modules, }); } @@ -489,9 +499,7 @@ function useEsbuild(props: { // on build errors anyway // so this is a no-op error handler }); - return () => { - result?.stop(); - }; + return () => result.stop?.(); }, [entry, destination, staticRoot, jsxFactory, jsxFragment]); return bundle; } @@ -505,7 +513,7 @@ function useWorker(props: { apiToken: string; bindings: CfWorkerInit["bindings"]; sitesFolder: undefined | string; - port: number; + port: undefined | number; compatibilityDate: string | undefined; compatibilityFlags: string[] | undefined; usageModel: undefined | "bundled" | "unbound"; @@ -631,7 +639,7 @@ function useProxy({ }: { token: CfPreviewToken | undefined; publicRoot: undefined | string; - port: number; + port: undefined | number; }) { useEffect(() => { if (!token) return; @@ -697,7 +705,7 @@ const SLEEP_DURATION = 2000; // really need a first class api for this const hostNameRegex = /userHostname="(.*)"/g; async function findTunnelHostname() { - let hostName: string; + let hostName: string | undefined; while (!hostName) { try { const resp = await fetch("http://localhost:8789/metrics"); diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index b5b1f1af2340..904ec3cf9a0d 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -72,10 +72,14 @@ async function readConfig(configPath?: string): Promise { "usage_model", ]; - Object.keys(config.env || {}).forEach((env) => { + const environments = config.env ?? {}; + Object.values(environments).forEach((env) => { + if (env === undefined) { + return; + } inheritedFields.forEach((field) => { - if (config[field] !== undefined && config.env[env][field] === undefined) { - config.env[env][field] = config[field]; // TODO: - shallow copy? + if (config[field] !== undefined && env[field] === undefined) { + env[field] = config[field]; // TODO: - shallow copy? } }); }); @@ -86,13 +90,16 @@ async function readConfig(configPath?: string): Promise { "durable_objects", "experimental_services", ]; - Object.keys(config.env || {}).forEach((env) => { + Object.entries(environments).forEach(([envName, envValue]) => { + if (envValue === undefined) { + return; + } mirroredFields.forEach((field) => { // if it exists on top level, it should exist on env definitions Object.keys(config[field] || {}).forEach((fieldKey) => { - if (!(fieldKey in config.env[env][field])) { + if (!(fieldKey in envValue[field])) { console.warn( - `In your configuration, "${field}.${fieldKey}" exists at a top level, but not on "env.${env}". This is not what you probably want, since the field "${field}" is not inherited by environments. Please add "${field}.${fieldKey}" to "env.${env}".` + `In your configuration, "${field}.${fieldKey}" exists at a top level, but not on "env.${envName}". This is not what you probably want, since the field "${field}" is not inherited by environments. Please add "${field}.${fieldKey}" to "env.${envName}".` ); } }); @@ -421,7 +428,11 @@ export async function main(argv: string[]): Promise { "👂 Start a local server for developing your worker", (yargs) => { return yargs - .positional("filename", { describe: "entry point", type: "string" }) + .positional("filename", { + describe: "entry point", + type: "string", + demandOption: true, + }) .option("name", { describe: "name of the script", type: "string", @@ -527,7 +538,8 @@ export async function main(argv: string[]): Promise { // -- snip, end -- - const envRootObj = args.env ? config.env[args.env] || {} : config; + const environments = config.env ?? {}; + const envRootObj = args.env ? environments[args.env] || {} : config; // TODO: this error shouldn't actually happen, // but we haven't fixed it internally yet @@ -768,6 +780,12 @@ export async function main(argv: string[]): Promise { // TODO: filter by client ip, which can be 'self' or an ip address }, async (args) => { + if (args.local) { + throw new NotImplementedError( + `local mode is not yet supported for this command` + ); + } + const config = args.config as Config; if (!(args.name || config.name)) { @@ -779,18 +797,16 @@ export async function main(argv: string[]): Promise { // -- snip, extract -- - if (!args.local) { - const loggedIn = await loginOrRefreshIfRequired(); - if (!loggedIn) { - // didn't login, let's just quit - console.log("Did not login, quitting..."); - return; - } + const loggedIn = await loginOrRefreshIfRequired(); + if (!loggedIn) { + // didn't login, let's just quit + console.log("Did not login, quitting..."); + return; + } + if (!config.account_id) { + config.account_id = await getAccountId(); if (!config.account_id) { - config.account_id = await getAccountId(); - if (!config.account_id) { - throw new Error("No account id found, quitting..."); - } + throw new Error("No account id found, quitting..."); } } @@ -1259,18 +1275,16 @@ export async function main(argv: string[]): Promise { // -- snip, extract -- - if (!args.local) { - const loggedIn = await loginOrRefreshIfRequired(); - if (!loggedIn) { - // didn't login, let's just quit - console.log("Did not login, quitting..."); - return; - } + const loggedIn = await loginOrRefreshIfRequired(); + if (!loggedIn) { + // didn't login, let's just quit + console.log("Did not login, quitting..."); + return; + } + if (!config.account_id) { + config.account_id = await getAccountId(); if (!config.account_id) { - config.account_id = await getAccountId(); - if (!config.account_id) { - throw new Error("No account id found, quitting..."); - } + throw new Error("No account id found, quitting..."); } } @@ -1309,18 +1323,16 @@ export async function main(argv: string[]): Promise { // -- snip, extract -- - if (!args.local) { - const loggedIn = await loginOrRefreshIfRequired(); - if (!loggedIn) { - // didn't login, let's just quit - console.log("Did not login, quitting..."); - return; - } + const loggedIn = await loginOrRefreshIfRequired(); + if (!loggedIn) { + // didn't login, let's just quit + console.log("Did not login, quitting..."); + return; + } + if (!config.account_id) { + config.account_id = await getAccountId(); if (!config.account_id) { - config.account_id = await getAccountId(); - if (!config.account_id) { - throw new Error("No account id found, quitting..."); - } + throw new Error("No account id found, quitting..."); } } @@ -1379,19 +1391,17 @@ export async function main(argv: string[]): Promise { // -- snip, extract -- - if (!args.local) { - const loggedIn = await loginOrRefreshIfRequired(); - if (!loggedIn) { - // didn't login, let's just quit - console.log("Did not login, quitting..."); - return; - } + const loggedIn = await loginOrRefreshIfRequired(); + if (!loggedIn) { + // didn't login, let's just quit + console.log("Did not login, quitting..."); + return; + } + if (!config.account_id) { + config.account_id = await getAccountId(); if (!config.account_id) { - config.account_id = await getAccountId(); - if (!config.account_id) { - throw new Error("No account id found, quitting..."); - } + throw new Error("No account id found, quitting..."); } } @@ -1438,6 +1448,7 @@ export async function main(argv: string[]): Promise { .positional("key", { type: "string", describe: "The key to write to.", + demandOption: true, }) .positional("value", { type: "string", @@ -1477,9 +1488,11 @@ export async function main(argv: string[]): Promise { }, async ({ key, ttl, expiration, ...args }) => { const namespaceId = getNamespaceId(args); + // One of `args.path` and `args.value` must be defined const value = args.path ? await readFile(args.path, "utf-8") - : args.value; + : // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + args.value!; const config = args.config as Config; if (args.path) { @@ -1501,11 +1514,7 @@ export async function main(argv: string[]): Promise { }); const ns = await mf.getKVNamespace(namespaceId); await ns.put(key, value, { expiration, expirationTtl: ttl }); - return; - } - // -- snip, extract -- - - if (!args.local) { + } else { const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1519,14 +1528,12 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } - } - // -- snip, end -- - - await putKeyValue(config.account_id, namespaceId, key, value, { - expiration, - expiration_ttl: ttl, - }); + await putKeyValue(config.account_id, namespaceId, key, value, { + expiration, + expiration_ttl: ttl, + }); + } } ) .command( @@ -1574,12 +1581,7 @@ export async function main(argv: string[]): Promise { const ns = await mf.getKVNamespace(namespaceId); const listResponse = await ns.list({ prefix }); console.log(JSON.stringify(listResponse.keys, null, " ")); // TODO: paginate, collate - return; - } - - // -- snip, extract -- - - if (!args.local) { + } else { const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1593,13 +1595,10 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } + console.log( + await listNamespaceKeys(config.account_id, namespaceId, prefix) + ); } - - // -- snip, end -- - - console.log( - await listNamespaceKeys(config.account_id, namespaceId, prefix) - ); } ) .command( @@ -1610,6 +1609,7 @@ export async function main(argv: string[]): Promise { .positional("key", { describe: "The key value to get.", type: "string", + demandOption: true, }) .option("binding", { type: "string", @@ -1686,6 +1686,7 @@ export async function main(argv: string[]): Promise { .positional("key", { describe: "The key value to delete", type: "string", + demandOption: true, }) .option("binding", { type: "string", @@ -1769,6 +1770,7 @@ export async function main(argv: string[]): Promise { .positional("filename", { describe: `The JSON file of key-value pairs to upload, in form [{"key":..., "value":...}"...]`, type: "string", + demandOption: true, }) .option("binding", { type: "string", @@ -1824,13 +1826,7 @@ export async function main(argv: string[]): Promise { expirationTtl: expiration_ttl, }); } - - return; - } - - // -- snip, extract -- - - if (!args.local) { + } else { const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1844,13 +1840,11 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } - } - - // -- snip, end -- - console.log( - await putBulkKeyValue(config.account_id, namespaceId, content) - ); + console.log( + await putBulkKeyValue(config.account_id, namespaceId, content) + ); + } } ) .command( @@ -1861,6 +1855,7 @@ export async function main(argv: string[]): Promise { .positional("filename", { describe: `The JSON file of key-value pairs to upload, in form ["key1", "key2", ...]`, type: "string", + demandOption: true, }) .option("binding", { type: "string", @@ -1904,13 +1899,7 @@ export async function main(argv: string[]): Promise { for (const key of parsedContent) { await ns.delete(key); } - - return; - } - - // -- snip, extract -- - - if (!args.local) { + } else { const loggedIn = await loginOrRefreshIfRequired(); if (!loggedIn) { // didn't login, let's just quit @@ -1924,13 +1913,15 @@ export async function main(argv: string[]): Promise { throw new Error("No account id found, quitting..."); } } - } - // -- snip, end -- - - console.log( - await deleteBulkKeyValue(config.account_id, namespaceId, content) - ); + console.log( + await deleteBulkKeyValue( + config.account_id, + namespaceId, + content + ) + ); + } } ); } diff --git a/packages/wrangler/src/inspect.ts b/packages/wrangler/src/inspect.ts index 68c9865062e2..d599d7c34ae8 100644 --- a/packages/wrangler/src/inspect.ts +++ b/packages/wrangler/src/inspect.ts @@ -60,7 +60,7 @@ export default function useInspector(props: InspectorProps) { const wsServerRef = useRef(); /** The websocket from the devtools instance. */ - const [localWebSocket, setLocalWebSocket] = useState(); + const [localWebSocket, setLocalWebSocket] = useState(); /** The websocket from the edge */ const [remoteWebSocket, setRemoteWebSocket] = useState< WebSocket | undefined @@ -78,7 +78,7 @@ export default function useInspector(props: InspectorProps) { res.end( JSON.stringify({ Browser: `wrangler/v${version}`, - // TODO: (someday): The DevTools protocol should match that of edgeworker. + // TODO: (someday): The DevTools protocol should match that of Edge Worker. // This could be exposed by the preview API. "Protocol-Version": "1.3", }) @@ -124,8 +124,8 @@ export default function useInspector(props: InspectorProps) { server: serverRef.current, clientTracking: true, }); - wsServerRef.current.on("connection", (ws: WebSocket) => { - if (wsServerRef.current.clients.size > 1) { + wsServerRef.current.on("connection", function (ws: WebSocket) { + if (this.clients.size > 1) { /** We only want to have one active Devtools instance at a time. */ console.error( "Tried to open a new devtools window when a previous one was already open." @@ -148,13 +148,19 @@ export default function useInspector(props: InspectorProps) { * of the component lifecycle. Convenient. */ useEffect(() => { - serverRef.current.listen(props.port); - return () => { - serverRef.current.close(); - // Also disconnect any open websockets/devtools connections - wsServerRef.current.clients.forEach((ws) => ws.close()); - wsServerRef.current.close(); - }; + if (serverRef.current) { + serverRef.current.listen(props.port); + return () => { + if (serverRef.current) { + serverRef.current.close(); + } + // Also disconnect any open websockets/devtools connections + if (wsServerRef.current) { + wsServerRef.current.clients.forEach((ws) => ws.close()); + wsServerRef.current.close(); + } + }; + } }, [props.port]); /** @@ -238,7 +244,7 @@ export default function useInspector(props: InspectorProps) { "🚨", // cheesy, but it works // maybe we could use color here too. params.exceptionDetails.text, - params.exceptionDetails.exception.description + params.exceptionDetails.exception?.description ?? "" ); } if (evt.method === "Runtime.consoleAPICalled") { @@ -284,7 +290,7 @@ export default function useInspector(props: InspectorProps) { clearInterval(keepAliveInterval); // Then we'll send a message to the devtools instance to // tell it to clear the console. - wsServerRef.current.clients.forEach((client) => { + wsServerRef.current?.clients.forEach((client) => { // We could've used `localSocket` here, but // then we would have had to add it to the effect // change detection array, which would have made a @@ -350,7 +356,7 @@ export default function useInspector(props: InspectorProps) { /** Send a message from the local websocket to the remote websocket */ function sendMessageToRemoteWebSocket(event: MessageEvent) { try { - remoteWebSocket.send(event.data); + remoteWebSocket?.send(event.data); } catch (e) { if (e.message !== "WebSocket is not open: readyState 0 (CONNECTING)") { /** @@ -367,7 +373,7 @@ export default function useInspector(props: InspectorProps) { /** Send a message from the local websocket to the remote websocket */ function sendMessageToLocalWebSocket(event: MessageEvent) { - localWebSocket.send(event.data); + localWebSocket?.send(event.data); } if (localWebSocket && remoteWebSocket) { @@ -420,7 +426,7 @@ function randomId(): string { * directly in the terminal. */ function logConsoleMessage(evt: Protocol.Runtime.ConsoleAPICalledEvent): void { - const args = []; + const args: string[] = []; for (const ro of evt.args) { switch (ro.type) { case "string": @@ -432,13 +438,13 @@ function logConsoleMessage(evt: Protocol.Runtime.ConsoleAPICalledEvent): void { args.push(ro.value); break; case "function": - args.push(`[Function: ${ro.description}]`); + args.push(`[Function: ${ro.description ?? ""}]`); break; case "object": if (!ro.preview) { - args.push(ro.description); + args.push(ro.description ?? ""); } else { - args.push(ro.preview.description); + args.push(ro.preview.description ?? ""); switch (ro.preview.subtype) { case "array": @@ -458,8 +464,10 @@ function logConsoleMessage(evt: Protocol.Runtime.ConsoleAPICalledEvent): void { args.push( "{\n" + ro.preview.entries - .map(({ key, value }) => { - return ` ${key.description} => ${value.description}`; + ?.map(({ key, value }) => { + return ` ${key?.description ?? ""} => ${ + value.description + }`; }) .join(",\n") + (ro.preview.overflow ? "\n ..." : "") + @@ -471,7 +479,7 @@ function logConsoleMessage(evt: Protocol.Runtime.ConsoleAPICalledEvent): void { args.push( "{ " + ro.preview.entries - .map(({ value }) => { + ?.map(({ value }) => { return `${value.description}`; }) .join(", ") + diff --git a/packages/wrangler/src/kv.tsx b/packages/wrangler/src/kv.tsx index 8b84033baf58..edd28be1ef5d 100644 --- a/packages/wrangler/src/kv.tsx +++ b/packages/wrangler/src/kv.tsx @@ -82,7 +82,7 @@ export interface NamespaceKeyInfo { export async function listNamespaceKeys( accountId: string, namespaceId: string, - prefix?: string + prefix = "" ) { return await fetchListResult( `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/keys`, @@ -98,15 +98,20 @@ export async function putKeyValue( value: string, args?: { expiration?: number; expiration_ttl?: number } ) { + let searchParams: URLSearchParams | undefined; + if (args) { + searchParams = new URLSearchParams(); + if (args.expiration) { + searchParams.set("expiration", `${args.expiration}`); + } + if (args.expiration_ttl) { + searchParams.set("expiration_ttl", `${args.expiration_ttl}`); + } + } return await fetchResult( `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/values/${key}`, { method: "PUT", body: value }, - args - ? new URLSearchParams({ - expiration: args.expiration?.toString(), - expiration_ttl: args.expiration_ttl?.toString(), - }) - : undefined + searchParams ); } @@ -262,6 +267,10 @@ export function getNamespaceId({ /** * KV namespace binding names must be valid JS identifiers. */ -export function isValidNamespaceBinding(binding: string): boolean { - return /^[a-zA-Z_][a-zA-Z0-9_]*$/.test(binding); +export function isValidNamespaceBinding( + binding: string | undefined +): binding is string { + return ( + typeof binding === "string" && /^[a-zA-Z_][a-zA-Z0-9_]*$/.test(binding) + ); } diff --git a/packages/wrangler/src/pages.tsx b/packages/wrangler/src/pages.tsx index f257ed6de800..9601ab4c0379 100644 --- a/packages/wrangler/src/pages.tsx +++ b/packages/wrangler/src/pages.tsx @@ -22,7 +22,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; @@ -862,11 +862,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 +1031,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." + ); +}; diff --git a/packages/wrangler/src/proxy.ts b/packages/wrangler/src/proxy.ts index c3f607d4852e..a54cb74f64c1 100644 --- a/packages/wrangler/src/proxy.ts +++ b/packages/wrangler/src/proxy.ts @@ -47,7 +47,7 @@ export function createHttpProxy(init: HttpProxyInit): Server { } const request = message.pipe(remote.request(headers)); request.on("response", (responseHeaders) => { - const status = responseHeaders[":status"]; + const status = responseHeaders[":status"] ?? 0; onResponse(responseHeaders); for (const name of Object.keys(responseHeaders)) { if (name.startsWith(":")) { diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index e08ac01f6264..16efeb5d1fbe 100644 --- a/packages/wrangler/src/publish.ts +++ b/packages/wrangler/src/publish.ts @@ -50,13 +50,18 @@ export default async function publish(props: Props): Promise { __path__, } = config; - const envRootObj = props.env ? config.env[props.env] || {} : config; + const envRootObj = + props.env && config.env ? config.env[props.env] || {} : config; assert( envRootObj.compatibility_date || props["compatibility-date"], "A compatibility_date is required when publishing. Add one to your wrangler.toml file, or pass it in your terminal as --compatibility_date. See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information." ); + if (accountId === undefined) { + throw new Error("No account_id provided."); + } + const triggers = props.triggers || config.triggers?.crons; const routes = props.routes || config.routes; @@ -129,18 +134,23 @@ export default async function publish(props: Props): Promise { ...(jsxFragment && { jsxFragment }), }); - const chunks = Object.entries(result.metafile.outputs).find( - ([_path, { entryPoint }]) => - entryPoint === - (props.public - ? path.join(path.dirname(file), "static-asset-facade.js") - : file) + const expectedEntryPoint = props.public + ? path.join(path.dirname(file), "static-asset-facade.js") + : file; + // result.metafile is defined because of the `metafile: true` option above. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const outputEntry = Object.entries(result.metafile!.outputs).find( + ([, { entryPoint }]) => entryPoint === expectedEntryPoint ); - + if (outputEntry === undefined) { + throw new Error( + `Cannot find entry-point "${expectedEntryPoint}" in generated bundle.` + ); + } const { format } = props; const bundle = { - type: chunks[1].exports.length > 0 ? "esm" : "commonjs", - exports: chunks[1].exports, + type: outputEntry[1].exports.length > 0 ? "esm" : "commonjs", + exports: outputEntry[1].exports, }; // TODO: instead of bundling the facade with the worker, we should just bundle the worker and expose it as a module. @@ -157,13 +167,13 @@ export default async function publish(props: Props): Promise { return; } - const content = await readFile(chunks[0], { encoding: "utf-8" }); + const content = await readFile(outputEntry[0], { encoding: "utf-8" }); await destination.cleanup(); // if config.migrations // get current migration tag let migrations; - if ("migrations" in config) { + if ("migrations" in config && config.migrations !== undefined) { const scripts = await fetchResult<{ id: string; migration_tag: string }[]>( `/accounts/${accountId}/workers/scripts` ); @@ -199,15 +209,10 @@ export default async function publish(props: Props): Promise { } } - const assets = - props.public || props.site || props.config.site?.bucket // TODO: allow both - ? await syncAssets( - accountId, - scriptName, - props.public || props.site || props.config.site?.bucket, - false - ) - : { manifest: undefined, namespace: undefined }; + const assetPath = props.public || props.site || props.config.site?.bucket; // TODO: allow both + const assets = assetPath + ? await syncAssets(accountId, scriptName, assetPath, false) + : { manifest: undefined, namespace: undefined }; const bindings: CfWorkerInit["bindings"] = { kv_namespaces: envRootObj.kv_namespaces?.concat( @@ -223,7 +228,7 @@ export default async function publish(props: Props): Promise { const worker: CfWorkerInit = { name: scriptName, main: { - name: path.basename(chunks[0]), + name: path.basename(outputEntry[0]), content: content, type: bundle.type === "esm" ? "esm" : "commonjs", }, diff --git a/packages/wrangler/src/sites.tsx b/packages/wrangler/src/sites.tsx index 1d9a00d48771..1fbb15360004 100644 --- a/packages/wrangler/src/sites.tsx +++ b/packages/wrangler/src/sites.tsx @@ -70,6 +70,12 @@ async function createKVNamespaceIfNotAlreadyExisting( }; } +interface UploadEntry { + key: string; + value: string; + base64: boolean; +} + export async function syncAssets( accountId: string, scriptName: string, @@ -89,7 +95,7 @@ export async function syncAssets( ); const manifest = {}; - const upload = []; + const upload: UploadEntry[] = []; // TODO: this can be more efficient by parallelising for await (const file of getFilesInFolder(dirPath)) { // TODO: "exclude:" config diff --git a/packages/wrangler/src/user.tsx b/packages/wrangler/src/user.tsx index e66d37c7c3ed..1d9b01fd2470 100644 --- a/packages/wrangler/src/user.tsx +++ b/packages/wrangler/src/user.tsx @@ -298,9 +298,9 @@ let initialised = false; // we do this because we have some async stuff // TODO: this should just happen in the top level -// abd we should fiure out how to do top level await +// abd we should figure out how to do top level await export async function initialise(): Promise { - // get refreshtoken/accesstoken from fs if exists + // get refreshToken/accessToken from fs if exists try { // if CF_API_TOKEN available, use that if (process.env.CF_API_TOKEN) { @@ -350,7 +350,9 @@ export function getAPIToken(): string { } throwIfNotInitialised(); - return LocalState.accessToken?.value; + // `throwIfNotInitialised()` ensures that the accessToken is guaranteed to be defined. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return LocalState.accessToken!.value; } interface AccessContext { @@ -971,14 +973,14 @@ export async function getAccountId() { }); } catch (err) { // probably offline + return; } if (!response) return; - let accountId: string; - // @ts-expect-error need to type this response - const responseJSON: { + let accountId: string | undefined; + const responseJSON = (await response.json()) as { success: boolean; result: { id: string; account: { id: string; name: string } }[]; - } = await response.json(); + }; if (responseJSON.success === true) { if (responseJSON.result.length === 1) { diff --git a/tsconfig.json b/tsconfig.json index 939fdad33335..65e40b047cd8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -10,7 +10,8 @@ "noEmit": true, "lib": ["esnext"], "jsx": "react", - "resolveJsonModule": true + "resolveJsonModule": true, + "strictNullChecks": true }, "exclude": ["node_modules/", "packages/wrangler/vendor"] }