From b5f42c587300c313bdebab4d364d0c7759e39752 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 15 Feb 2022 16:27:50 +0000 Subject: [PATCH] chore: enable `strict` in `tsconfig.json` (#456) In the march towards full strictness, this enables `strict` in `tsconfig.json` and fixes the errors it pops up. A changeset is included because there are some subtle code changes, and we should leave a trail for them. --- .changeset/modern-owls-begin.md | 7 ++ .../src/__tests__/helpers/command-exists.d.ts | 6 ++ .../src/__tests__/helpers/faye-websocket.d.ts | 6 ++ .../src/__tests__/helpers/mock-bin.ts | 5 +- .../src/__tests__/helpers/run-wrangler.ts | 8 ++- packages/wrangler/src/__tests__/kv.test.ts | 6 +- .../src/__tests__/package-manager.test.ts | 4 +- .../wrangler/src/__tests__/publish.test.ts | 2 +- .../wrangler/src/__tests__/secret.test.ts | 6 +- packages/wrangler/src/api/worker.ts | 5 +- packages/wrangler/src/cfetch/internal.ts | 11 +++- packages/wrangler/src/config.ts | 12 +++- packages/wrangler/src/dev.tsx | 14 ++-- packages/wrangler/src/index.tsx | 18 ++++-- packages/wrangler/src/inspect.ts | 6 +- packages/wrangler/src/pages.tsx | 11 ++-- packages/wrangler/src/publish.ts | 2 +- packages/wrangler/src/sites.tsx | 2 +- packages/wrangler/src/user.tsx | 64 +++++++++++-------- tsconfig.json | 2 +- 20 files changed, 132 insertions(+), 65 deletions(-) create mode 100644 .changeset/modern-owls-begin.md create mode 100644 packages/wrangler/src/__tests__/helpers/command-exists.d.ts create mode 100644 packages/wrangler/src/__tests__/helpers/faye-websocket.d.ts diff --git a/.changeset/modern-owls-begin.md b/.changeset/modern-owls-begin.md new file mode 100644 index 000000000000..89278028ba73 --- /dev/null +++ b/.changeset/modern-owls-begin.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +chore: enable `strict` in `tsconfig.json` + +In the march towards full strictness, this enables `strict` in `tsconfig.json` and fixes the errors it pops up. A changeset is included because there are some subtle code changes, and we should leave a trail for them. diff --git a/packages/wrangler/src/__tests__/helpers/command-exists.d.ts b/packages/wrangler/src/__tests__/helpers/command-exists.d.ts new file mode 100644 index 000000000000..7596c7935fb6 --- /dev/null +++ b/packages/wrangler/src/__tests__/helpers/command-exists.d.ts @@ -0,0 +1,6 @@ +module "command-exists" { + /** + * Detect whether a command exists on the system. + */ + export default function commandExists(cmd: string): Promise; +} diff --git a/packages/wrangler/src/__tests__/helpers/faye-websocket.d.ts b/packages/wrangler/src/__tests__/helpers/faye-websocket.d.ts new file mode 100644 index 000000000000..145593215836 --- /dev/null +++ b/packages/wrangler/src/__tests__/helpers/faye-websocket.d.ts @@ -0,0 +1,6 @@ +module "faye-websocket" { + /** + * Standards-compliant WebSocket client and server. + */ + export default WebSocket; +} diff --git a/packages/wrangler/src/__tests__/helpers/mock-bin.ts b/packages/wrangler/src/__tests__/helpers/mock-bin.ts index 83f19bac3d15..233272683924 100644 --- a/packages/wrangler/src/__tests__/helpers/mock-bin.ts +++ b/packages/wrangler/src/__tests__/helpers/mock-bin.ts @@ -7,7 +7,10 @@ const nodeShebang = "#!/usr/bin/env node"; /** * Create a binary file in a temp directory and make it available on the PATH. */ -export async function mockBinary(binaryName: string, code: string) { +export async function mockBinary( + binaryName: string, + code: string +): Promise<() => void> { // Ensure there is a directory to put the mock binary in. const tmpDir = resolve(mkdtempSync(".mock-binary-")); diff --git a/packages/wrangler/src/__tests__/helpers/run-wrangler.ts b/packages/wrangler/src/__tests__/helpers/run-wrangler.ts index 1fc8dbc73a1e..a59e88d4d238 100644 --- a/packages/wrangler/src/__tests__/helpers/run-wrangler.ts +++ b/packages/wrangler/src/__tests__/helpers/run-wrangler.ts @@ -7,8 +7,10 @@ import { normalizeSlashes, stripTimings } from "./mock-console"; export async function runWrangler(cmd?: string) { try { await main(cmd?.split(" ") ?? []); - } catch (e) { - e.message = normalizeSlashes(stripTimings(e.message)); - throw e; + } catch (err) { + if (err instanceof Error) { + err.message = normalizeSlashes(stripTimings(err.message)); + } + throw err; } } diff --git a/packages/wrangler/src/__tests__/kv.test.ts b/packages/wrangler/src/__tests__/kv.test.ts index 69e59a9b9772..d09a48ce50ac 100644 --- a/packages/wrangler/src/__tests__/kv.test.ts +++ b/packages/wrangler/src/__tests__/kv.test.ts @@ -10,7 +10,7 @@ import { mockConsoleMethods } from "./helpers/mock-console"; import { mockKeyListRequest } from "./helpers/mock-kv"; import { runInTempDir } from "./helpers/run-in-tmp"; import { runWrangler } from "./helpers/run-wrangler"; -import type { KVNamespaceInfo } from "../kv"; +import type { KVNamespaceInfo, NamespaceKeyInfo } from "../kv"; describe("wrangler", () => { mockAccountId(); @@ -839,7 +839,9 @@ describe("wrangler", () => { "kv:key list --namespace-id some-namespace-id --limit 100" ); expect(std.err).toMatchInlineSnapshot(`""`); - expect(JSON.parse(std.out).map((k) => k.name)).toEqual(keys); + expect( + JSON.parse(std.out).map((k: NamespaceKeyInfo) => k.name) + ).toEqual(keys); expect(requests.count).toEqual(6); }); }); diff --git a/packages/wrangler/src/__tests__/package-manager.test.ts b/packages/wrangler/src/__tests__/package-manager.test.ts index 02e79165a446..1587c1a691fc 100644 --- a/packages/wrangler/src/__tests__/package-manager.test.ts +++ b/packages/wrangler/src/__tests__/package-manager.test.ts @@ -151,7 +151,7 @@ describe("getPackageManager()", () => { * Create a fake yarn binary */ function mockYarn(succeed: boolean): void { - let unMock; + let unMock: () => void; beforeEach(async () => { unMock = await mockBinary("yarn", `process.exit(${succeed ? 0 : 1})`); }); @@ -162,7 +162,7 @@ function mockYarn(succeed: boolean): void { * Create a fake npm binary */ function mockNpm(succeed: boolean): void { - let unMock; + let unMock: () => void; beforeEach(async () => { unMock = await mockBinary("npm", `process.exit(${succeed ? 0 : 1})`); }); diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts index 231cad81a226..5cf914168500 100644 --- a/packages/wrangler/src/__tests__/publish.test.ts +++ b/packages/wrangler/src/__tests__/publish.test.ts @@ -321,7 +321,7 @@ export default{ expect(std.out).toMatchInlineSnapshot(`""`); expect(std.err).toMatchInlineSnapshot(` - "A [site] definition requires a \`bucket\` field with a path to the site's public directory. + "AssertionError [ERR_ASSERTION]: A [site] definition requires a \`bucket\` field with a path to the site's public directory. %s If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new." diff --git a/packages/wrangler/src/__tests__/secret.test.ts b/packages/wrangler/src/__tests__/secret.test.ts index c39804ee0fa2..f0f6e2b047ba 100644 --- a/packages/wrangler/src/__tests__/secret.test.ts +++ b/packages/wrangler/src/__tests__/secret.test.ts @@ -54,7 +54,7 @@ describe("wrangler secret", () => { try { await runWrangler("secret put the-key"); } catch (e) { - error = e; + error = e as Error; } expect(std.out).toMatchInlineSnapshot(`""`); expect(std.err).toMatchInlineSnapshot(` @@ -120,7 +120,7 @@ describe("wrangler secret", () => { try { await runWrangler("secret delete the-key"); } catch (e) { - error = e; + error = e as Error; } expect(std.out).toMatchInlineSnapshot(`""`); expect(std.err).toMatchInlineSnapshot(` @@ -186,7 +186,7 @@ describe("wrangler secret", () => { try { await runWrangler("secret list"); } catch (e) { - error = e; + error = e as Error; } expect(std.out).toMatchInlineSnapshot(`""`); expect(std.err).toMatchInlineSnapshot(` diff --git a/packages/wrangler/src/api/worker.ts b/packages/wrangler/src/api/worker.ts index fb546ac2234a..52d3fe13cc18 100644 --- a/packages/wrangler/src/api/worker.ts +++ b/packages/wrangler/src/api/worker.ts @@ -114,7 +114,10 @@ export interface CfDurableObjectMigrations { new_tag: string; steps: { new_classes?: string[]; - renamed_classes?: string[]; + renamed_classes?: { + from: string; + to: string; + }[]; deleted_classes?: string[]; }[]; } diff --git a/packages/wrangler/src/cfetch/internal.ts b/packages/wrangler/src/cfetch/internal.ts index 1ab772cc57c8..1acc63fac86f 100644 --- a/packages/wrangler/src/cfetch/internal.ts +++ b/packages/wrangler/src/cfetch/internal.ts @@ -42,7 +42,9 @@ export async function fetchInternal( } } -function cloneHeaders(headers: HeadersInit | undefined): HeadersInit { +function cloneHeaders( + headers: HeadersInit | undefined +): Record { return { ...headers }; } @@ -61,8 +63,11 @@ function requireApiToken(): string { return apiToken; } -function addAuthorizationHeader(headers: HeadersInit, apiToken: string): void { - if (headers["Authorization"]) { +function addAuthorizationHeader( + headers: Record, + apiToken: string +): void { + if ("Authorization" in headers) { throw new Error( "The request already specifies an authorisation header - cannot add a new one." ); diff --git a/packages/wrangler/src/config.ts b/packages/wrangler/src/config.ts index ecc0e002baf0..6881a7ae2b03 100644 --- a/packages/wrangler/src/config.ts +++ b/packages/wrangler/src/config.ts @@ -535,7 +535,13 @@ export function normaliseAndValidateEnvironmentsConfig(config: Config) { ); // Fall back on "inherited fields" from the config, if not specified in the environment. - const inheritedFields = [ + + type InheritedField = keyof Omit< + Config, + "env" | "migrations" | "wasm_modules" | "site" | "dev" + >; + + const inheritedFields: InheritedField[] = [ "name", "account_id", "workers_dev", @@ -546,7 +552,6 @@ export function normaliseAndValidateEnvironmentsConfig(config: Config) { "route", "jsx_factory", "jsx_fragment", - "site", "triggers", "usage_model", ]; @@ -554,7 +559,8 @@ export function normaliseAndValidateEnvironmentsConfig(config: Config) { for (const inheritedField of inheritedFields) { if (config[inheritedField] !== undefined) { if (environment[inheritedField] === undefined) { - environment[inheritedField] = config[inheritedField]; // TODO: - shallow or deep copy? + (environment[inheritedField] as typeof environment[InheritedField]) = + config[inheritedField]; // TODO: - shallow or deep copy? } } } diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index 191ac6c43976..134bbf8f22f1 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -26,6 +26,7 @@ import type { CfModule, CfWorkerInit, CfScriptFormat } from "./api/worker"; import type { Entry } from "./bundle"; import type { AssetPaths } from "./sites"; import type { WatchMode } from "esbuild"; +import type { ExecaChildProcess } from "execa"; import type { DirectoryResult } from "tmp-promise"; export type DevProps = { @@ -454,7 +455,8 @@ function useCustomBuild( const { command, cwd, watch_dir } = props; useEffect(() => { if (!command) return; - let cmd, interval; + let cmd: ExecaChildProcess | undefined, + interval: NodeJS.Timeout | undefined; console.log("running:", command); cmd = execaCommand(command, { ...(cwd && { cwd }), @@ -467,7 +469,7 @@ function useCustomBuild( "all", (_event, filePath) => { console.log(`The file ${filePath} changed, restarting build...`); - cmd.kill(); + cmd?.kill(); cmd = execaCommand(command, { ...(cwd && { cwd }), shell: true, @@ -494,7 +496,7 @@ function useCustomBuild( } if (fileExists === true) { - clearInterval(interval); + interval && clearInterval(interval); setEntry(expectedEntry); } else { const elapsed = Date.now() - startedAt; @@ -503,8 +505,8 @@ function useCustomBuild( console.error( `⎔ Build timed out, Could not resolve ${expectedEntry.file}` ); - clearInterval(interval); - cmd.kill(); + interval && clearInterval(interval); + cmd?.kill(); } } }, 200); @@ -514,7 +516,7 @@ function useCustomBuild( cmd.kill(); cmd = undefined; } - clearInterval(interval); + interval && clearInterval(interval); interval = undefined; }; }, [command, cwd, expectedEntry, watch_dir]); diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index c0b8c0763bf8..19138dedff1f 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -111,7 +111,7 @@ ${TOML.stringify({ if (configPath && "wasm_modules" in config) { // rewrite wasm_module paths to be absolute - const modules = {}; + const modules: Record = {}; for (const [name, filePath] of Object.entries(config.wasm_modules || {})) { modules[name] = path.relative( process.cwd(), @@ -332,7 +332,7 @@ export async function main(argv: string[]): Promise { // TODO: suggest next steps? } catch (err) { throw new Error( - `Failed to create wrangler.toml.\n${err.message ?? err}` + `Failed to create wrangler.toml.\n${(err as Error).message ?? err}` ); } } @@ -1373,6 +1373,7 @@ export async function main(argv: string[]): Promise { try { await submitSecret(); } catch (e) { + // @ts-expect-error non-standard property on Error if (e.code === 10007) { // upload a draft worker await fetchResult( @@ -1724,7 +1725,8 @@ export async function main(argv: string[]): Promise { id = getNamespaceId(args, config); } catch (e) { throw new CommandLineArgsError( - "Not able to delete namespace.\n" + e.message + "Not able to delete namespace.\n" + + ((e as Error).message ?? e) ); } @@ -2140,7 +2142,9 @@ export async function main(argv: string[]): Promise { parsedContent = JSON.parse(content); } catch (err) { throw new Error( - `Could not parse json from ${filename}.\n${err.message ?? err}` + `Could not parse json from ${filename}.\n${ + (err as Error).message ?? err + }` ); } @@ -2224,7 +2228,9 @@ export async function main(argv: string[]): Promise { parsedContent = JSON.parse(content); } catch (err) { throw new Error( - `Could not parse json from ${filename}.\n${err.message ?? err}` + `Could not parse json from ${filename}.\n${ + (err as Error).message ?? err + }` ); } @@ -2431,7 +2437,7 @@ export async function main(argv: string[]): Promise { console.error(""); // Just adds a bit of space console.error(e.message); } else { - console.error(e.message); + console.error(e instanceof Error ? e.message : e); console.error(""); // Just adds a bit of space console.error( `${fgGreenColor}%s${resetColor}`, diff --git a/packages/wrangler/src/inspect.ts b/packages/wrangler/src/inspect.ts index bdd6e030b43e..0f671fd2341e 100644 --- a/packages/wrangler/src/inspect.ts +++ b/packages/wrangler/src/inspect.ts @@ -367,7 +367,10 @@ export default function useInspector(props: InspectorProps) { ); remoteWebSocket.send(event.data); } catch (e) { - if (e.message !== "WebSocket is not open: readyState 0 (CONNECTING)") { + if ( + (e as Error).message !== + "WebSocket is not open: readyState 0 (CONNECTING)" + ) { /** * ^ this just means we haven't opened a websocket yet * usually happens until there's at least one request @@ -587,6 +590,7 @@ function logConsoleMessage(evt: Protocol.Runtime.ConsoleAPICalledEvent): void { const method = mapConsoleAPIMessageTypeToConsoleMethod[evt.type]; if (method in console) { + // @ts-expect-error - we know this is a valid method // eslint-disable-next-line prefer-spread console[method].apply(console, args); } else { diff --git a/packages/wrangler/src/pages.tsx b/packages/wrangler/src/pages.tsx index 5922905549ef..31747c74a485 100644 --- a/packages/wrangler/src/pages.tsx +++ b/packages/wrangler/src/pages.tsx @@ -795,8 +795,8 @@ export const pages: BuilderCallback = (yargs) => { let miniflareArgs: MiniflareOptions = {}; - let scriptReadyResolve; - const scriptReadyPromise = new Promise( + let scriptReadyResolve: () => void; + const scriptReadyPromise = new Promise( (resolve) => (scriptReadyResolve = resolve) ); @@ -830,7 +830,8 @@ export const pages: BuilderCallback = (yargs) => { scriptPath, }; } else { - scriptReadyResolve(); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + scriptReadyResolve!(); const scriptPath = directory !== undefined @@ -900,7 +901,7 @@ export const pages: BuilderCallback = (yargs) => { // env.ASSETS.fetch serviceBindings: { - async ASSETS(request) { + async ASSETS(request: Request) { if (proxyPort) { try { const url = new URL(request.url); @@ -962,7 +963,7 @@ export const pages: BuilderCallback = (yargs) => { miniflare.dispose().catch((err) => miniflare.log.error(err)); }); } catch (e) { - miniflare.log.error(e); + miniflare.log.error(e as Error); EXIT("Could not start Miniflare.", 1); } } diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index c802710b69d7..6488f1adcae6 100644 --- a/packages/wrangler/src/publish.ts +++ b/packages/wrangler/src/publish.ts @@ -222,7 +222,7 @@ export default async function publish(props: Props): Promise { type: bundleType, }, bindings, - ...(migrations && { migrations }), + migrations, modules, compatibility_date: config.compatibility_date, compatibility_flags: config.compatibility_flags, diff --git a/packages/wrangler/src/sites.tsx b/packages/wrangler/src/sites.tsx index 5d5e062c651e..883e4f918b2b 100644 --- a/packages/wrangler/src/sites.tsx +++ b/packages/wrangler/src/sites.tsx @@ -129,7 +129,7 @@ export async function syncAssets( const result = await listNamespaceKeys(accountId, namespace); const keys = new Set(result.map((x) => x.name)); - const manifest = {}; + const manifest: Record = {}; const upload: { key: string; value: string; diff --git a/packages/wrangler/src/user.tsx b/packages/wrangler/src/user.tsx index e03b6614edc1..0e38649e1d2e 100644 --- a/packages/wrangler/src/user.tsx +++ b/packages/wrangler/src/user.tsx @@ -222,6 +222,7 @@ import React from "react"; import { fetch } from "undici"; import { CF_API_BASE_URL } from "./cfetch"; import openInBrowser from "./open-in-browser"; +import type { Item as SelectInputItem } from "ink-select-input/build/SelectInput"; import type { ParsedUrlQuery } from "node:querystring"; import type { Response } from "undici"; @@ -276,14 +277,14 @@ const Scopes = { * * "offline_access" is automatically included. */ -type Scope = keyof typeof Scopes | "offline_access"; +type Scope = keyof typeof Scopes; const ScopeKeys = Object.keys(Scopes) as Scope[]; export function validateScopeKeys( scopes: string[] ): scopes is typeof ScopeKeys { - return scopes.every((scope) => Scopes[scope]); + return scopes.every((scope) => scope in Scopes); } const CLIENT_ID = "54d11594-84e4-41aa-b438-e81b8fa78ee7"; @@ -561,6 +562,7 @@ export async function getAuthURL(scopes = ScopeKeys): Promise { `?response_type=code&` + `client_id=${encodeURIComponent(CLIENT_ID)}&` + `redirect_uri=${encodeURIComponent(CALLBACK_URL)}&` + + // @ts-expect-error we add offline_access manually `scope=${encodeURIComponent(scopes.concat("offline_access").join(" "))}&` + `state=${stateQueryParam}&` + `code_challenge=${encodeURIComponent(codeChallenge)}&` + @@ -568,6 +570,17 @@ export async function getAuthURL(scopes = ScopeKeys): Promise { ); } +type TokenResponse = + | { + access_token: string; + expires_in: number; + refresh_token: string; + scope: string; + } + | { + error: string; + }; + /** * Refresh an access token from the remote service. */ @@ -592,13 +605,12 @@ async function exchangeRefreshTokenForAccessToken(): Promise { throw await response.json(); } else { try { - const json = await response.json(); - const { access_token, expires_in, refresh_token, scope } = json as { - access_token: string; - expires_in: number; - refresh_token: string; - scope: string; - }; + const json = (await response.json()) as TokenResponse; + if ("error" in json) { + throw json.error; + } + + const { access_token, expires_in, refresh_token, scope } = json; let scopes: Scope[] = []; const accessToken: AccessToken = { @@ -626,20 +638,24 @@ async function exchangeRefreshTokenForAccessToken(): Promise { refreshToken: LocalState.refreshToken, }; return accessContext; - } catch (err) { - const error = err?.error || "There was a network error."; + } catch (error) { switch (error) { case "invalid_grant": - console.log( + console.warn( "Expired! Auth code or refresh token needs to be renewed." ); // alert("Redirecting to auth server to obtain a new auth grant code."); // TODO: return refreshAuthCodeOrRefreshToken(); break; default: + console.error(error); break; } - throw toErrorClass(error); + if (typeof error === "string") { + throw toErrorClass(error); + } else { + throw error; + } } } } @@ -680,13 +696,11 @@ async function exchangeAuthCodeForAccessToken(): Promise { } throw toErrorClass(error); } - const json = await response.json(); - const { access_token, expires_in, refresh_token, scope } = json as { - access_token: string; - expires_in: number; - refresh_token: string; - scope: string; - }; + const json = (await response.json()) as TokenResponse; + if ("error" in json) { + throw new Error(json.error); + } + const { access_token, expires_in, refresh_token, scope } = json; let scopes: Scope[] = []; LocalState.hasAuthCodeBeenExchangedForAccessToken = true; @@ -805,8 +819,8 @@ export async function loginOrRefreshIfRequired(): Promise { export async function login(props?: LoginProps): Promise { const urlToOpen = await getAuthURL(props?.scopes); await openInBrowser(urlToOpen); - let server; - let loginTimeoutHandle; + let server: http.Server; + let loginTimeoutHandle: NodeJS.Timeout; const timerPromise = new Promise((resolve) => { loginTimeoutHandle = setTimeout(() => { console.error( @@ -847,7 +861,7 @@ export async function login(props?: LoginProps): Promise { }); console.log( "Error: Consent denied. You must grant consent to Wrangler in order to login. If you don't want to do this consider passing an API token with CF_API_TOKEN variable" - ); // TODO: implement wrangler config lol + ); return; } else { @@ -939,7 +953,7 @@ export async function logout(): Promise { export function listScopes(): void { throwIfNotInitialised(); console.log("💁 Available scopes:"); - const data = ScopeKeys.map((scope) => ({ + const data = ScopeKeys.map((scope: keyof typeof Scopes) => ({ Scope: scope, Description: Scopes[scope], })); @@ -1001,7 +1015,7 @@ type ChooseAccountItem = { }; export function ChooseAccount(props: { accounts: ChooseAccountItem[]; - onSelect: (item) => void; + onSelect: (item: SelectInputItem) => void; }) { return ( <> diff --git a/tsconfig.json b/tsconfig.json index 732557e3d865..a0f94d5abe4e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -8,7 +8,7 @@ "noEmit": true, "lib": ["esnext"], "incremental": true, - "strictNullChecks": true, + "strict": true, "skipLibCheck": true }, "exclude": ["node_modules/"]