[Performance] Memoize isShopify - #7711
Conversation
Memoize the result of the `isShopify` function to avoid redundant filesystem checks for the developer binary. The result is cached as a Promise when using the default `process.env`.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/context/local.d.ts@@ -33,6 +33,11 @@ export declare function isVerbose(env?: NodeJS.ProcessEnv): boolean;
* @returns True if the CLI is used in a Shopify environment.
*/
export declare function isShopify(env?: NodeJS.ProcessEnv): Promise<boolean>;
+/**
+ * Resets the memoized value for the Shopify check.
+ * This is only used for testing purposes.
+ */
+export declare function _resetIsShopifyMemo(): void;
/**
* This variable is used when running unit tests to indicate that the CLI's business logic
* is run as a subject of a unit test. We can use this variable to disable output through
|
WHY are these changes introduced?
The
isShopifyfunction is called frequently, including during analytics generation for every command. It performs an asynchronous filesystem check for the presence of thedevbinary. Since the environment and installation status are static during a single CLI execution, this check can be safely memoized to improve efficiency.WHAT is this pull request doing?
isShopifyresult when called with the defaultprocess.env.Promisedirectly to ensure concurrent calls await the same operation._resetIsShopifyMemoutility to maintain test isolation.How to test your changes?
pnpm --filter @shopify/cli-kit vitest run src/public/node/context/local.test.tsPost-release steps
None.
Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset addPR created automatically by Jules for task 14669418771078202875 started by @gonzaloriestra