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 #300

.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

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<PagesConfigCache>(pagesConfigCacheFilename)
+		).toMatchInlineSnapshot(`Object {}`);
+	});
+
+	it("should ignore attempts to cache values ", () => {
+		saveToConfigCache<PagesConfigCache>(pagesConfigCacheFilename, {
+			account_id: "some-account-id",
+			pages_project_name: "foo",
+		});
+		expect(getConfigCache<PagesConfigCache>(pagesConfigCacheFilename)).toEqual(
+			{}
+		);
+
+		saveToConfigCache<PagesConfigCache>(pagesConfigCacheFilename, {
+			pages_project_name: "bar",
+		});
+		expect(getConfigCache<PagesConfigCache>(pagesConfigCacheFilename)).toEqual(
+			{}
+		);
+	});
+});

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";
 

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 = <T>(fileName: string): Partial<T> => {
+export function getConfigCache<T>(fileName: string): Partial<T> {
 	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 = <T>(
+export function saveToConfigCache<T>(
 	fileName: string,
 	newValues: Partial<T>
-) => {
-	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 });
+	}
+}

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<DevArgs>) {
 					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}

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 ? (
 		<ChooseAccount
 			accounts={accountChoices}
-			onSelect={(selectedAccountId) => setAccountId(selectedAccountId)}
+			onSelect={(selectedAccount) => {
+				saveAccountToCache(selectedAccount);
+				setAccountId(selectedAccount.id);
+			}}
 			onError={(err) => errorHandler(err)}
 		></ChooseAccount>
 	) : null;

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;
+	}
+}

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 });
 				}}
 			/>
 		</>

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<string | undefined> {
 	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(
-				<ChooseAccount
-					accounts={accounts}
-					onSelect={async (selected) => {
-						resolve(selected);
-						unmount();
-					}}
-					onError={(err) => {
-						reject(err);
-						unmount();
-					}}
-				/>
-			);
-		});
+		const account = await new Promise<{ id: string; name: string }>(
+			(resolve, reject) => {
+				const { unmount } = render(
+					<ChooseAccount
+						accounts={accounts}
+						onSelect={async (selected) => {
+							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;
 }