From 562d3adf908541e52be6e463a120f7acbc6ef8ee Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sun, 2 Jan 2022 21:30:01 +0000 Subject: [PATCH] enable eslint's `no-shadow` rule (#180) * enable eslint's `no-shadow` rule This PR enables eslint's `no-shadow` rule. I started a refactor of a couple of pieces and found myself getting confused with vars that claimed to be defined without it being so. The `no-shadow` rule prevents vars from being defined in inner scopes if they've already been defined in an outer scope. This PR is mostly clean. It did lead to some wordy vars in `index.tsx`, but that's fine, it only made them more obvious. I didn't bother doing this in `pages.tsx`, I'll leave that up to the pages team. Instead, I've just disabled the rule for that file. * add a changeset * stray commented out line * Apply suggested changes from review --- .changeset/large-guests-retire.md | 5 ++ package.json | 1 + packages/wrangler/src/api/form_data.ts | 9 +-- packages/wrangler/src/dev.tsx | 30 ++++----- packages/wrangler/src/dialogs.tsx | 4 +- packages/wrangler/src/index.tsx | 92 +++++++++++++------------- packages/wrangler/src/kv.tsx | 4 +- packages/wrangler/src/pages.tsx | 2 + packages/wrangler/src/proxy.ts | 37 +++++------ packages/wrangler/src/publish.ts | 2 +- packages/wrangler/src/user.tsx | 23 +++---- 11 files changed, 102 insertions(+), 107 deletions(-) create mode 100644 .changeset/large-guests-retire.md diff --git a/.changeset/large-guests-retire.md b/.changeset/large-guests-retire.md new file mode 100644 index 000000000000..3b307efc5772 --- /dev/null +++ b/.changeset/large-guests-retire.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +chore: enable eslint's no-shadow rule diff --git a/package.json b/package.json index dfddf5e538d7..a6eb06dc0a93 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "@typescript-eslint/consistent-type-imports": [ "error" ], + "no-shadow": "error", "@typescript-eslint/no-floating-promises": "error", "no-empty": "off", "require-yield": "off", diff --git a/packages/wrangler/src/api/form_data.ts b/packages/wrangler/src/api/form_data.ts index df7b921d93b4..90a3fb4fad9b 100644 --- a/packages/wrangler/src/api/form_data.ts +++ b/packages/wrangler/src/api/form_data.ts @@ -67,7 +67,6 @@ export function toFormData(worker: CfWorkerInit): FormData { compatibility_date, compatibility_flags, } = worker; - const { name, type: mainType } = main; const metadataBindings: WorkerMetadata["bindings"] = []; @@ -104,7 +103,9 @@ export function toFormData(worker: CfWorkerInit): FormData { }); const metadata: WorkerMetadata = { - ...(mainType !== "commonjs" ? { main_module: name } : { body_part: name }), + ...(main.type !== "commonjs" + ? { main_module: main.name } + : { body_part: main.name }), bindings: metadataBindings, ...(compatibility_date && { compatibility_date }), ...(compatibility_flags && { compatibility_flags }), @@ -114,7 +115,7 @@ export function toFormData(worker: CfWorkerInit): FormData { formData.set("metadata", JSON.stringify(metadata)); - if (mainType === "commonjs" && modules && modules.length > 0) { + if (main.type === "commonjs" && modules && modules.length > 0) { throw new TypeError( "More than one module can only be specified when type = 'esm'" ); @@ -122,7 +123,7 @@ export function toFormData(worker: CfWorkerInit): FormData { for (const module of [main].concat(modules || [])) { const { name } = module; - const blob = toModule(module, mainType ?? "esm"); + const blob = toModule(module, main.type ?? "esm"); formData.set(name, blob, name); } diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index 7a8e1fb48fb5..bf671bfd8090 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -491,7 +491,10 @@ function useEsbuild(props: { else { // nothing really changes here, so let's increment the id // to change the return object's identity - setBundle((bundle) => ({ ...bundle, id: bundle.id + 1 })); + setBundle((previousBundle) => ({ + ...previousBundle, + id: previousBundle.id + 1, + })); } }, }, @@ -818,31 +821,24 @@ function useHotkeys(initial: useHotkeysInitialState, port: number) { ) => { switch (input) { case "b": // open browser - await open( - `http://localhost:${port}/` - // { - // app: { - // name: open.apps.chrome, // TODO: fallback on other browsers - // }, - // } - ); + await open(`http://localhost:${port}/`); break; case "d": // toggle inspector await open( `https://built-devtools.pages.dev/js_app?experiments=true&v8only=true&ws=localhost:9229/ws` - // { - // app: { - // name: open.apps.chrome, - // // todo - add firefox and edge fallbacks - // }, - // } ); break; case "s": // toggle tunnel - setToggles((toggles) => ({ ...toggles, tunnel: !toggles.tunnel })); + setToggles((previousToggles) => ({ + ...previousToggles, + tunnel: !previousToggles.tunnel, + })); break; case "l": // toggle local - setToggles((toggles) => ({ ...toggles, local: !toggles.local })); + setToggles((previousToggles) => ({ + ...previousToggles, + local: !previousToggles.local, + })); break; case "q": // shut down case "x": // shut down diff --git a/packages/wrangler/src/dialogs.tsx b/packages/wrangler/src/dialogs.tsx index 69a3ce074377..e831c154bd05 100644 --- a/packages/wrangler/src/dialogs.tsx +++ b/packages/wrangler/src/dialogs.tsx @@ -67,9 +67,9 @@ export async function prompt(text: string, type: "text" | "password" = "text") { { + onSubmit={(inputText) => { unmount(); - resolve(text); + resolve(inputText); }} /> ); diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index fcfdee044516..f991bf65c592 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -3,7 +3,7 @@ import { render } from "ink"; import Dev from "./dev"; import { readFile } from "node:fs/promises"; import makeCLI from "yargs"; -import type yargs from "yargs"; +import type Yargs from "yargs"; import { findUp } from "find-up"; import TOML from "@iarna/toml"; import type { Config } from "./config"; @@ -43,15 +43,15 @@ import { setTimeout } from "node:timers/promises"; import * as fs from "node:fs"; import { execa } from "execa"; -async function readConfig(path?: string): Promise { +async function readConfig(configPath?: string): Promise { const config: Config = {}; - if (!path) { - path = await findUp("wrangler.toml"); + if (!configPath) { + configPath = await findUp("wrangler.toml"); // TODO - terminate this early instead of going all the way to the root } - if (path) { - const tml: string = await readFile(path, "utf-8"); + if (configPath) { + const tml: string = await readFile(configPath, "utf-8"); const parsed = TOML.parse(tml) as Config; Object.assign(config, parsed); } @@ -109,7 +109,7 @@ async function readConfig(path?: string): Promise { // let's just do some basics for now // @ts-expect-error we're being sneaky here for now - config.__path__ = path; + config.__path__ = configPath; return config; } @@ -117,7 +117,7 @@ async function readConfig(path?: string): Promise { // a helper to demand one of a set of options // via https://github.com/yargs/yargs/issues/1093#issuecomment-491299261 function demandOneOfOption(...options: string[]) { - return function (argv: yargs.Arguments) { + return function (argv: Yargs.Arguments) { const count = options.filter((option) => argv[option]).length; const lastOption = options.pop(); @@ -144,7 +144,7 @@ class DeprecationError extends Error {} class NotImplementedError extends Error {} export async function main(argv: string[]): Promise { - const yargs = makeCLI(argv) + const wrangler = makeCLI(argv) // We handle errors ourselves in a try-catch around `yargs.parse`. // If you want the "help info" to be displayed then throw an instance of `CommandLineArgsError`. // Otherwise we just log the error that was thrown without any "help info". @@ -160,7 +160,7 @@ export async function main(argv: string[]): Promise { .wrap(null); // the default is to simply print the help menu - yargs.command( + wrangler.command( ["*"], false, () => {}, @@ -168,7 +168,7 @@ export async function main(argv: string[]): Promise { if (args._.length > 0) { throw new CommandLineArgsError(`Unknown command: ${args._}.`); } else { - yargs.showHelp("log"); + wrangler.showHelp("log"); } } ); @@ -181,7 +181,7 @@ export async function main(argv: string[]): Promise { // (It's also annoying that choices[] doesn't get inferred as an enum. 🤷‍♂.) // [DEPRECATED] generate - yargs.command( + wrangler.command( // we definitely want to move away from us cloning github templates // we can do something better here, let's see "generate [name] [template]", @@ -206,7 +206,7 @@ export async function main(argv: string[]): Promise { ); // init - yargs.command( + wrangler.command( "init [name]", "📥 Create a wrangler.toml configuration file", (yargs) => { @@ -320,7 +320,7 @@ export async function main(argv: string[]): Promise { ); // build - yargs.command( + wrangler.command( "build", false, (yargs) => { @@ -337,7 +337,7 @@ export async function main(argv: string[]): Promise { ); // login - yargs.command( + wrangler.command( // this needs scopes as an option? "login", false, // we don't need to show this in the menu @@ -380,7 +380,7 @@ export async function main(argv: string[]): Promise { ); // logout - yargs.command( + wrangler.command( // this needs scopes as an option? "logout", false, // we don't need to show this in the menu @@ -392,7 +392,7 @@ export async function main(argv: string[]): Promise { ); // whoami - yargs.command( + wrangler.command( "whoami", false, // we don't need to show this the menu // "🕵️ Retrieve your user info and test your auth config", @@ -403,7 +403,7 @@ export async function main(argv: string[]): Promise { ); // config - yargs.command( + wrangler.command( "config", false, () => {}, @@ -416,7 +416,7 @@ export async function main(argv: string[]): Promise { ); // dev - yargs.command( + wrangler.command( "dev ", "👂 Start a local server for developing your worker", (yargs) => { @@ -562,7 +562,7 @@ export async function main(argv: string[]): Promise { ); // publish - yargs.command( + wrangler.command( "publish [script]", "🆙 Publish your Worker to Cloudflare.", (yargs) => { @@ -654,7 +654,7 @@ export async function main(argv: string[]): Promise { ); // tail - yargs.command( + wrangler.command( "tail [name]", "🦚 Starts a log tailing session for a deployed Worker.", (yargs) => { @@ -770,7 +770,7 @@ export async function main(argv: string[]): Promise { ); // preview - yargs.command( + wrangler.command( "preview [method] [body]", false, (yargs) => { @@ -804,12 +804,12 @@ export async function main(argv: string[]): Promise { ); // route - yargs.command( + wrangler.command( "route", false, // I think we want to hide this command // "➡️ List or delete worker routes", - (yargs) => { - return yargs + (routeYargs) => { + return routeYargs .command( "list", "List a route associated with a zone", @@ -876,7 +876,7 @@ export async function main(argv: string[]): Promise { ); // subdomain - yargs.command( + wrangler.command( "subdomain [name]", false, // "👷 Create or change your workers.dev subdomain.", @@ -891,11 +891,11 @@ export async function main(argv: string[]): Promise { ); // secret - yargs.command( + wrangler.command( "secret", "🤫 Generate a secret that can be referenced in the worker script", - (yargs) => { - return yargs + (secretYargs) => { + return secretYargs .command( "put ", "Create or update a secret variable for a script", @@ -1126,11 +1126,11 @@ export async function main(argv: string[]): Promise { // kv // :namespace - yargs.command( + wrangler.command( "kv:namespace", "🗂️ Interact with your Workers KV Namespaces", - (yargs) => { - return yargs + (namespaceYargs) => { + return namespaceYargs .command( "create ", "Create a new namespace", @@ -1361,11 +1361,11 @@ export async function main(argv: string[]): Promise { ); // :key - yargs.command( + wrangler.command( "kv:key", "🔑 Individually manage Workers KV key-value pairs", - (yargs) => { - return yargs + (kvKeyYargs) => { + return kvKeyYargs .command( "put [value]", "Writes a single key/value pair to the given namespace.", @@ -1684,11 +1684,11 @@ export async function main(argv: string[]): Promise { ); // :bulk - yargs.command( + wrangler.command( "kv:bulk", "💪 Interact with multiple Workers KV key-value pairs at once", - (yargs) => { - return yargs + (kvBulkYargs) => { + return kvBulkYargs .command( "put ", "Upload multiple key-value pairs to a namespace", @@ -1864,9 +1864,9 @@ export async function main(argv: string[]): Promise { } ); - yargs.command("pages", "⚡️ Configure Cloudflare Pages", pages); + wrangler.command("pages", "⚡️ Configure Cloudflare Pages", pages); - yargs + wrangler .option("config", { alias: "c", describe: "Path to .toml configuration file", @@ -1882,18 +1882,18 @@ export async function main(argv: string[]): Promise { default: false, // I bet this will a point of contention. We'll revisit it. }); - yargs.group(["config", "help", "version"], "Flags:"); - yargs.help().alias("h", "help"); - yargs.version(wranglerVersion).alias("v", "version"); - yargs.exitProcess(false); + wrangler.group(["config", "help", "version"], "Flags:"); + wrangler.help().alias("h", "help"); + wrangler.version(wranglerVersion).alias("v", "version"); + wrangler.exitProcess(false); await initialiseUserConfig(); try { - await yargs.parse(); + await wrangler.parse(); } catch (e) { if (e instanceof CommandLineArgsError) { - yargs.showHelp("error"); + wrangler.showHelp("error"); console.error(""); // Just adds a bit of space } console.error(e.message); diff --git a/packages/wrangler/src/kv.tsx b/packages/wrangler/src/kv.tsx index 251bc8e0ef61..5a753d92c260 100644 --- a/packages/wrangler/src/kv.tsx +++ b/packages/wrangler/src/kv.tsx @@ -190,9 +190,7 @@ export function getNamespaceId({ ); } - const namespace = config.kv_namespaces.find( - (namespace) => namespace.binding === binding - ); + const namespace = config.kv_namespaces.find((ns) => ns.binding === binding); // we couldn't find a namespace with that binding if (!namespace) { diff --git a/packages/wrangler/src/pages.tsx b/packages/wrangler/src/pages.tsx index 9385865fcf0c..cc4718aaa78a 100644 --- a/packages/wrangler/src/pages.tsx +++ b/packages/wrangler/src/pages.tsx @@ -1,3 +1,5 @@ +/* eslint-disable no-shadow */ + import type { BuilderCallback } from "yargs"; import { join } from "path"; import { tmpdir } from "os"; diff --git a/packages/wrangler/src/proxy.ts b/packages/wrangler/src/proxy.ts index b3a8d7ac0f30..c3f607d4852e 100644 --- a/packages/wrangler/src/proxy.ts +++ b/packages/wrangler/src/proxy.ts @@ -1,11 +1,6 @@ import { connect } from "node:http2"; import { createServer } from "node:http"; -import type { - Server, - IncomingHttpHeaders, - OutgoingHttpHeaders, - RequestListener, -} from "node:http"; +import type { Server, IncomingHttpHeaders, RequestListener } from "node:http"; import WebSocket from "faye-websocket"; import serveStatic from "serve-static"; @@ -13,7 +8,7 @@ export interface HttpProxyInit { host: string; assetPath?: string | null; onRequest?: (headers: IncomingHttpHeaders) => void; - onResponse?: (headers: OutgoingHttpHeaders) => void; + onResponse?: (headers: IncomingHttpHeaders) => void; } /** @@ -28,9 +23,9 @@ export function createHttpProxy(init: HttpProxyInit): Server { onRequest(headers); headers[":authority"] = host; const request = stream.pipe(remote.request(headers)); - request.on("response", (headers: OutgoingHttpHeaders) => { - onResponse(headers); - stream.respond(headers); + request.on("response", (responseHeaders: IncomingHttpHeaders) => { + onResponse(responseHeaders); + stream.respond(responseHeaders); request.pipe(stream, { end: true }); }); }); @@ -51,15 +46,15 @@ export function createHttpProxy(init: HttpProxyInit): Server { } } const request = message.pipe(remote.request(headers)); - request.on("response", (headers) => { - const status = headers[":status"]; - onResponse(headers); - for (const name of Object.keys(headers)) { + request.on("response", (responseHeaders) => { + const status = responseHeaders[":status"]; + onResponse(responseHeaders); + for (const name of Object.keys(responseHeaders)) { if (name.startsWith(":")) { - delete headers[name]; + delete responseHeaders[name]; } } - response.writeHead(status, headers); + response.writeHead(status, responseHeaders); request.pipe(response, { end: true }); }); }; @@ -82,10 +77,14 @@ export function createHttpProxy(init: HttpProxyInit): Server { const { headers, url } = message; onRequest(headers); headers["host"] = host; - const local = new WebSocket(message, socket, body); + const localWebsocket = new WebSocket(message, socket, body); // TODO(soon): Custom WebSocket protocol is not working? - const remote = new WebSocket.Client(`wss://${host}${url}`, [], { headers }); - local.pipe(remote).pipe(local); + const remoteWebsocketClient = new WebSocket.Client( + `wss://${host}${url}`, + [], + { headers } + ); + localWebsocket.pipe(remoteWebsocketClient).pipe(localWebsocket); }); remote.on("close", () => { local.close(); diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index b16f1d2f5163..e80f40a4cc2c 100644 --- a/packages/wrangler/src/publish.ts +++ b/packages/wrangler/src/publish.ts @@ -158,7 +158,7 @@ export default async function publish(props: Props): Promise { const scripts = await cfetch<{ id: string; migration_tag: string }[]>( `/accounts/${accountId}/workers/scripts` ); - const script = scripts.find((script) => script.id === scriptName); + const script = scripts.find(({ id }) => id === scriptName); if (script?.migration_tag) { // was already published once const foundIndex = config.migrations.findIndex( diff --git a/packages/wrangler/src/user.tsx b/packages/wrangler/src/user.tsx index bb977be1c98e..e66d37c7c3ed 100644 --- a/packages/wrangler/src/user.tsx +++ b/packages/wrangler/src/user.tsx @@ -578,19 +578,16 @@ export async function getAuthURL(scopes?: string[]): Promise { * Refresh an access token from the remote service. */ async function exchangeRefreshTokenForAccessToken(): Promise { - const { refreshToken } = LocalState; - - if (!refreshToken) { + if (!LocalState.refreshToken) { console.warn("No refresh token is present."); } - const url = TOKEN_URL; const body = `grant_type=refresh_token&` + - `refresh_token=${refreshToken?.value}&` + + `refresh_token=${LocalState.refreshToken?.value}&` + `client_id=${CLIENT_ID}`; - const response = await fetch(url, { + const response = await fetch(TOKEN_URL, { method: "POST", body, headers: { @@ -617,10 +614,9 @@ async function exchangeRefreshTokenForAccessToken(): Promise { LocalState.accessToken = accessToken; if (refresh_token) { - const refreshToken: RefreshToken = { + LocalState.refreshToken = { value: refresh_token, }; - LocalState.refreshToken = refreshToken; } if (scope) { @@ -666,7 +662,6 @@ async function exchangeAuthCodeForAccessToken(): Promise { console.warn("No authorization grant code is being passed."); } - const url = TOKEN_URL; const body = `grant_type=authorization_code&` + `code=${encodeURIComponent(authorizationCode || "")}&` + @@ -674,7 +669,7 @@ async function exchangeAuthCodeForAccessToken(): Promise { `client_id=${encodeURIComponent(CLIENT_ID)}&` + `code_verifier=${codeVerifier}`; - const response = await fetch(url, { + const response = await fetch(TOKEN_URL, { method: "POST", body, headers: { @@ -709,10 +704,9 @@ async function exchangeAuthCodeForAccessToken(): Promise { LocalState.accessToken = accessToken; if (refresh_token) { - const refreshToken: RefreshToken = { + LocalState.refreshToken = { value: refresh_token, }; - LocalState.refreshToken = refreshToken; } if (scope) { @@ -921,15 +915,14 @@ export async function refreshToken(): Promise { export async function logout(): Promise { throwIfNotInitialised(); - const { refreshToken } = LocalState; - if (!refreshToken) { + if (!LocalState.refreshToken) { console.log("Not logged in, exiting..."); return; } const body = `client_id=${encodeURIComponent(CLIENT_ID)}&` + `token_type_hint=refresh_token&` + - `token=${encodeURIComponent(refreshToken?.value || "")}`; + `token=${encodeURIComponent(LocalState.refreshToken?.value || "")}`; const response = await fetch(REVOKE_URL, { method: "POST",