Skip to content

Commit

Permalink
feat: cache account id selection
Browse files Browse the repository at this point in the history
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_id 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
  • Loading branch information
threepointone committed Jul 4, 2022
1 parent ea7ee45 commit bfab7d0
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 34 deletions.
17 changes: 17 additions & 0 deletions .changeset/silent-ears-jam.md
Original file line number Diff line number Diff line change
@@ -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_id 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
Original file line number Diff line number Diff line change
@@ -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("cannot read and write 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(
{}
);
});
});
6 changes: 6 additions & 0 deletions packages/wrangler/src/__tests__/config-cache.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -8,6 +10,10 @@ interface PagesConfigCache {

describe("config cache", () => {
runInTempDir();
mockConsoleMethods();
beforeEach(() => {
mkdirSync("node_modules");
});

const pagesConfigCacheFilename = "pages-config-cache.json";

Expand Down
98 changes: 68 additions & 30 deletions packages/wrangler/src/config-cache.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,82 @@
import { mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
import { dirname, join } from "path";
import * as path from "path";
import { findUpSync } from "find-up";
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 = () => {
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) {
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;
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 });
}
}
7 changes: 6 additions & 1 deletion packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import getPort from "get-port";
import { render } from "ink";
import React from "react";
import { findWranglerToml, printBindings, readConfig } from "./config";
import { getConfigCache } from "./config-cache";
import Dev from "./dev/dev";
import { getVarsForDev } from "./dev/dev-vars";

Expand Down Expand Up @@ -475,7 +476,11 @@ export async function startDev(args: ArgumentsCamelCase<DevArgs>) {
enableLocalPersistence={
args["experimental-enable-local-persistence"] || false
}
accountId={config.account_id}
accountId={
config.account_id ||
getConfigCache<{ account_id: string }>("wrangler-account.json")
.account_id
}
assetPaths={assetPaths}
port={args.port || config.dev.port || (await getLocalPort())}
ip={args.ip || config.dev.ip}
Expand Down
16 changes: 14 additions & 2 deletions packages/wrangler/src/dev/remote.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import path from "node:path";
import React, { useState, useEffect, useRef } from "react";
import { useErrorHandler } from "react-error-boundary";
import { printBundleSize } from "../bundle-reporter";
import { saveToConfigCache } from "../config-cache";
import {
createPreviewSession,
createWorkerPreview,
Expand Down Expand Up @@ -96,13 +97,19 @@ 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) {
saveToConfigCache<{ account_id: string }>("wrangler-account.json", {
account_id: accounts[0].id,
});
setAccountId(accounts[0].id);
} else {
setAccountChoices(accounts);
Expand All @@ -119,7 +126,12 @@ export function Remote(props: {
return accountId === undefined && accountChoices !== undefined ? (
<ChooseAccount
accounts={accountChoices}
onSelect={(selectedAccountId) => setAccountId(selectedAccountId)}
onSelect={(selectedAccountId) => {
saveToConfigCache<{ account_id: string }>("wrangler-account.json", {
account_id: selectedAccountId,
});
setAccountId(selectedAccountId);
}}
onError={(err) => errorHandler(err)}
></ChooseAccount>
) : null;
Expand Down
19 changes: 18 additions & 1 deletion packages/wrangler/src/user/user.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,11 @@ 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 { logger } from "../logger";
import openInBrowser from "../open-in-browser";
import { parseTOML, readFileSync } from "../parse";
Expand Down Expand Up @@ -1058,8 +1062,18 @@ export async function getAccountId(): Promise<string | undefined> {
const apiToken = getAPIToken();
if (!apiToken) return;

// check if we have a cached value
const cachedAccount = getConfigCache<{ account_id: string }>(
"wrangler-account.json"
);
if (cachedAccount.account_id) {
return cachedAccount.account_id;
}
const accounts = await getAccountChoices();
if (accounts.length === 1) {
saveToConfigCache<{ account_id: string }>("wrangler-account.json", {
account_id: accounts[0].id,
});
return accounts[0].id;
}

Expand All @@ -1069,6 +1083,9 @@ export async function getAccountId(): Promise<string | undefined> {
<ChooseAccount
accounts={accounts}
onSelect={async (selected) => {
saveToConfigCache<{ account_id: string }>("wrangler-account.json", {
account_id: selected,
});
resolve(selected);
unmount();
}}
Expand Down

0 comments on commit bfab7d0

Please sign in to comment.