From 88f270223be22c74b6374f6eefdf8e9fbf798e4d Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 5 Jul 2022 11:43:25 +0100 Subject: [PATCH] feat: cache account id selection (#1395) This adds caching for account id fetch/selection for all wrangler commands. Currently, if we have an api/oauth token, but haven't provided an account id, we fetch account information from cloudflare. If a user has just one account id, we automatically choose that. If there are more than one, then we show a dropdown and ask the user to pick one. This is convenient, and lets the user not have to specify their account id when starting a project. However, if does make startup slow, since it has to do that fetch every time. It's also annoying for folks with multiple account ids because they have to pick their account id every time. So we now cache the account details into `node_modules/.cache/wrangler` (much like pages already does with account id and project name). This patch also refactors `config-cache.ts`; it only caches if there's a `node_modules` folder, and it looks for the closest node_modules folder (and not directly in cwd). I also added tests for when a `node_modules` folder isn't available. It also trims the message that we log to terminal. Closes https://github.com/cloudflare/wrangler2/issues/300 --- .changeset/silent-ears-jam.md | 17 +++ .../config-cache-without-cache-dir.test.ts | 38 +++++++ .../src/__tests__/config-cache.test.ts | 6 ++ packages/wrangler/src/config-cache.ts | 101 ++++++++++++------ packages/wrangler/src/dev.tsx | 3 +- packages/wrangler/src/dev/remote.tsx | 21 +++- packages/wrangler/src/is-interactive.ts | 12 +++ packages/wrangler/src/user/choose-account.tsx | 4 +- packages/wrangler/src/user/user.tsx | 75 +++++++++---- 9 files changed, 218 insertions(+), 59 deletions(-) create mode 100644 .changeset/silent-ears-jam.md create mode 100644 packages/wrangler/src/__tests__/config-cache-without-cache-dir.test.ts create mode 100644 packages/wrangler/src/is-interactive.ts diff --git a/.changeset/silent-ears-jam.md b/.changeset/silent-ears-jam.md new file mode 100644 index 000000000000..65425287855c --- /dev/null +++ b/.changeset/silent-ears-jam.md @@ -0,0 +1,17 @@ +--- +"wrangler": patch +--- + +feat: cache account id selection + +This adds caching for account id fetch/selection for all wrangler commands. + +Currently, if we have an api/oauth token, but haven't provided an account id, we fetch account information from cloudflare. If a user has just one account id, we automatically choose that. If there are more than one, then we show a dropdown and ask the user to pick one. This is convenient, and lets the user not have to specify their account id when starting a project. + +However, if does make startup slow, since it has to do that fetch every time. It's also annoying for folks with multiple account ids because they have to pick their account id every time. + +So we now cache the account details into `node_modules/.cache/wrangler` (much like pages already does with account id and project name). + +This patch also refactors `config-cache.ts`; it only caches if there's a `node_modules` folder, and it looks for the closest node_modules folder (and not directly in cwd). I also added tests for when a `node_modules` folder isn't available. It also trims the message that we log to terminal. + +Closes https://github.com/cloudflare/wrangler2/issues/300 diff --git a/packages/wrangler/src/__tests__/config-cache-without-cache-dir.test.ts b/packages/wrangler/src/__tests__/config-cache-without-cache-dir.test.ts new file mode 100644 index 000000000000..4f37d767850c --- /dev/null +++ b/packages/wrangler/src/__tests__/config-cache-without-cache-dir.test.ts @@ -0,0 +1,38 @@ +import { getConfigCache, saveToConfigCache } from "../config-cache"; +import { mockConsoleMethods } from "./helpers/mock-console"; +import { runInTempDir } from "./helpers/run-in-tmp"; + +interface PagesConfigCache { + account_id: string; + pages_project_name: string; +} + +describe("config cache", () => { + runInTempDir(); + mockConsoleMethods(); + // In this set of tests, we don't create a node_modules folder + const pagesConfigCacheFilename = "pages-config-cache.json"; + + it("should return an empty config if no file exists", () => { + expect( + getConfigCache(pagesConfigCacheFilename) + ).toMatchInlineSnapshot(`Object {}`); + }); + + it("should ignore attempts to cache values ", () => { + saveToConfigCache(pagesConfigCacheFilename, { + account_id: "some-account-id", + pages_project_name: "foo", + }); + expect(getConfigCache(pagesConfigCacheFilename)).toEqual( + {} + ); + + saveToConfigCache(pagesConfigCacheFilename, { + pages_project_name: "bar", + }); + expect(getConfigCache(pagesConfigCacheFilename)).toEqual( + {} + ); + }); +}); diff --git a/packages/wrangler/src/__tests__/config-cache.test.ts b/packages/wrangler/src/__tests__/config-cache.test.ts index 587747f6f270..8c86563e806f 100644 --- a/packages/wrangler/src/__tests__/config-cache.test.ts +++ b/packages/wrangler/src/__tests__/config-cache.test.ts @@ -1,4 +1,6 @@ +import { mkdirSync } from "node:fs"; import { getConfigCache, saveToConfigCache } from "../config-cache"; +import { mockConsoleMethods } from "./helpers/mock-console"; import { runInTempDir } from "./helpers/run-in-tmp"; interface PagesConfigCache { @@ -8,6 +10,10 @@ interface PagesConfigCache { describe("config cache", () => { runInTempDir(); + mockConsoleMethods(); + beforeEach(() => { + mkdirSync("node_modules"); + }); const pagesConfigCacheFilename = "pages-config-cache.json"; diff --git a/packages/wrangler/src/config-cache.ts b/packages/wrangler/src/config-cache.ts index f714c2dab4c8..8abf0a895b77 100644 --- a/packages/wrangler/src/config-cache.ts +++ b/packages/wrangler/src/config-cache.ts @@ -1,44 +1,83 @@ import { mkdirSync, readFileSync, rmSync, writeFileSync } from "fs"; -import { dirname, join } from "path"; +import * as path from "path"; +import { findUpSync } from "find-up"; +import isInteractive from "./is-interactive"; +import { logger } from "./logger"; let cacheMessageShown = false; -const cacheFolder = "node_modules/.cache/wrangler"; +let __cacheFolder: string | null; +function getCacheFolder() { + if (__cacheFolder || __cacheFolder === null) return __cacheFolder; -const showCacheMessage = () => { - if (!cacheMessageShown) { - console.log( - `Using cached values in '${cacheFolder}'. This is used as a temporary store to improve the developer experience for some commands. It may be purged at any time. It doesn't contain any sensitive information, but it should not be commited into source control.` - ); - cacheMessageShown = true; + const closestNodeModulesDirectory = findUpSync("node_modules", { + type: "directory", + }); + __cacheFolder = closestNodeModulesDirectory + ? path.join(closestNodeModulesDirectory, ".cache/wrangler") + : null; + + if (!__cacheFolder) { + logger.debug("No folder available to cache configuration"); + } + return __cacheFolder; +} + +const arrayFormatter = new Intl.ListFormat("en", { + style: "long", + type: "conjunction", +}); + +function showCacheMessage(fields: string[], folder: string) { + if (!cacheMessageShown && isInteractive()) { + if (fields.length > 0) { + logger.log( + `Retrieving cached values for ${arrayFormatter.format( + fields + )} from ${path.relative(process.cwd(), folder)}` + ); + cacheMessageShown = true; + } } -}; +} -export const getConfigCache = (fileName: string): Partial => { +export function getConfigCache(fileName: string): Partial { try { - const configCacheLocation = join(cacheFolder, fileName); - const configCache = JSON.parse(readFileSync(configCacheLocation, "utf-8")); - showCacheMessage(); - return configCache; - } catch { + const cacheFolder = getCacheFolder(); + if (cacheFolder) { + const configCacheLocation = path.join(cacheFolder, fileName); + const configCache = JSON.parse( + readFileSync(configCacheLocation, "utf-8") + ); + showCacheMessage(Object.keys(configCache), cacheFolder); + return configCache; + } else return {}; + } catch (err) { return {}; } -}; +} -export const saveToConfigCache = ( +export function saveToConfigCache( fileName: string, newValues: Partial -) => { - const configCacheLocation = join(cacheFolder, fileName); - const existingValues = getConfigCache(fileName); - - mkdirSync(dirname(configCacheLocation), { recursive: true }); - writeFileSync( - configCacheLocation, - JSON.stringify({ ...existingValues, ...newValues }, null, 2) - ); -}; - -export const purgeConfigCaches = () => { - rmSync(cacheFolder, { recursive: true, force: true }); -}; +): void { + const cacheFolder = getCacheFolder(); + if (cacheFolder) { + logger.debug(`Saving to cache: ${JSON.stringify(newValues)}`); + const configCacheLocation = path.join(cacheFolder, fileName); + const existingValues = getConfigCache(fileName); + + mkdirSync(path.dirname(configCacheLocation), { recursive: true }); + writeFileSync( + configCacheLocation, + JSON.stringify({ ...existingValues, ...newValues }, null, 2) + ); + } +} + +export function purgeConfigCaches() { + const cacheFolder = getCacheFolder(); + if (cacheFolder) { + rmSync(cacheFolder, { recursive: true, force: true }); + } +} diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index 7c07501b2808..1a83ad0d0e13 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -11,6 +11,7 @@ import { getVarsForDev } from "./dev/dev-vars"; import { getEntry } from "./entry"; import { logger } from "./logger"; import { getAssetPaths, getSiteAssetPaths } from "./sites"; +import { getAccountFromCache } from "./user"; import { getZoneIdFromHost, getZoneForRoute, getHostFromRoute } from "./zones"; import { printWranglerBanner, @@ -477,7 +478,7 @@ export async function startDev(args: ArgumentsCamelCase) { enableLocalPersistence={ args["experimental-enable-local-persistence"] || false } - accountId={config.account_id} + accountId={config.account_id || getAccountFromCache()?.id} assetPaths={assetPaths} port={args.port || config.dev.port || (await getLocalPort())} ip={args.ip || config.dev.ip} diff --git a/packages/wrangler/src/dev/remote.tsx b/packages/wrangler/src/dev/remote.tsx index b4f33049ff16..8bea793f3d2f 100644 --- a/packages/wrangler/src/dev/remote.tsx +++ b/packages/wrangler/src/dev/remote.tsx @@ -11,7 +11,12 @@ import useInspector from "../inspect"; import { logger } from "../logger"; import { usePreviewServer } from "../proxy"; import { syncAssets } from "../sites"; -import { ChooseAccount, getAccountChoices, requireApiToken } from "../user"; +import { + ChooseAccount, + getAccountChoices, + requireApiToken, + saveAccountToCache, +} from "../user"; import type { Route } from "../config/environment"; import type { CfPreviewToken, @@ -96,13 +101,20 @@ export function Remote(props: { // This effect handles the async step of fetching the available accounts for the current user. // If only one account is available then it is just used by calling `setAccountId()`. useEffect(() => { - if (accountChoicesRef.current !== undefined) { + if ( + accountChoicesRef.current !== undefined || + props.accountId !== undefined + ) { return; } accountChoicesRef.current = getAccountChoices(); accountChoicesRef.current.then( (accounts) => { if (accounts.length === 1) { + saveAccountToCache({ + id: accounts[0].id, + name: accounts[0].name, + }); setAccountId(accounts[0].id); } else { setAccountChoices(accounts); @@ -119,7 +131,10 @@ export function Remote(props: { return accountId === undefined && accountChoices !== undefined ? ( setAccountId(selectedAccountId)} + onSelect={(selectedAccount) => { + saveAccountToCache(selectedAccount); + setAccountId(selectedAccount.id); + }} onError={(err) => errorHandler(err)} > ) : null; diff --git a/packages/wrangler/src/is-interactive.ts b/packages/wrangler/src/is-interactive.ts new file mode 100644 index 000000000000..fa9ef5795be6 --- /dev/null +++ b/packages/wrangler/src/is-interactive.ts @@ -0,0 +1,12 @@ +/** + * Test whether the process is "interactive". + * Reasons it may not be interactive: it could be running in CI, + * or you're piping values from / to another process, etc + */ +export default function isInteractive(): boolean { + try { + return Boolean(process.stdin.isTTY && process.stdout.isTTY); + } catch { + return false; + } +} diff --git a/packages/wrangler/src/user/choose-account.tsx b/packages/wrangler/src/user/choose-account.tsx index 9c28d1ae148f..5d275220b0ec 100644 --- a/packages/wrangler/src/user/choose-account.tsx +++ b/packages/wrangler/src/user/choose-account.tsx @@ -15,7 +15,7 @@ export type ChooseAccountItem = { */ export function ChooseAccount(props: { accounts: ChooseAccountItem[]; - onSelect: (accountId: string) => void; + onSelect: (account: { name: string; id: string }) => void; onError: (error: Error) => void; }) { return ( @@ -29,7 +29,7 @@ export function ChooseAccount(props: { }))} onSelect={(item) => { logger.log(`Using account: "${item.value.name} - ${item.value.id}"`); - props.onSelect(item.value.id); + props.onSelect({ id: item.value.id, name: item.value.name }); }} /> diff --git a/packages/wrangler/src/user/user.tsx b/packages/wrangler/src/user/user.tsx index f8a95de6a779..18443655eb22 100644 --- a/packages/wrangler/src/user/user.tsx +++ b/packages/wrangler/src/user/user.tsx @@ -219,7 +219,12 @@ import { render } from "ink"; import Table from "ink-table"; import React from "react"; import { fetch } from "undici"; -import { purgeConfigCaches } from "../config-cache"; +import { + getConfigCache, + purgeConfigCaches, + saveToConfigCache, +} from "../config-cache"; +import isInteractive from "../is-interactive"; import { logger } from "../logger"; import openInBrowser from "../open-in-browser"; import { parseTOML, readFileSync } from "../parse"; @@ -1058,27 +1063,37 @@ export async function getAccountId(): Promise { const apiToken = getAPIToken(); if (!apiToken) return; + // check if we have a cached value + const cachedAccount = getAccountFromCache(); + if (cachedAccount) { + return cachedAccount.id; + } const accounts = await getAccountChoices(); if (accounts.length === 1) { + saveAccountToCache({ id: accounts[0].id, name: accounts[0].name }); return accounts[0].id; } if (isInteractive()) { - return await new Promise((resolve, reject) => { - const { unmount } = render( - { - resolve(selected); - unmount(); - }} - onError={(err) => { - reject(err); - unmount(); - }} - /> - ); - }); + const account = await new Promise<{ id: string; name: string }>( + (resolve, reject) => { + const { unmount } = render( + { + saveAccountToCache(selected); + resolve(selected); + unmount(); + }} + onError={(err) => { + reject(err); + unmount(); + }} + /> + ); + } + ); + return account.id; } throw new Error( @@ -1121,10 +1136,26 @@ export function requireApiToken(): ApiCredentials { return credentials; } -function isInteractive(): boolean { - try { - return Boolean(process.stdin.isTTY && process.stdout.isTTY); - } catch { - return false; - } +/** + * Save the given account details to a cache + */ +export function saveAccountToCache(account: { + id: string; + name: string; +}): void { + saveToConfigCache<{ account: { id: string; name: string } }>( + "wrangler-account.json", + { account } + ); +} + +/** + * Fetch the given account details from a cache if available + */ +export function getAccountFromCache(): + | undefined + | { id: string; name: string } { + return getConfigCache<{ account: { id: string; name: string } }>( + "wrangler-account.json" + ).account; }