[Performance] Memoize macAddress retrieval#7676
Conversation
This change memoizes the `macAddress` function in `packages/cli-kit/src/public/node/context/local.ts` to prevent redundant system calls during analytics payload generation. The retrieval of the MAC address is constant for the duration of a CLI command execution, so caching the promise ensures that subsequent calls are nearly instantaneous. - Added `memoizedMacAddress` module-level variable to `local.ts`. - Updated `macAddress()` to return the cached promise if available. - Updated `local.test.ts` to verify that `macaddress.one()` is only called once.
|
👋 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. |
github-actions
Bot
added
the
no-changelog
This PR improves Shopify CLI performance by memoizing the `macAddress` retrieval, which is used in every analytics report. It also upgrades `hashString` and `nonRandomUUID` to use SHA-256 instead of SHA-1 to address CodeQL security alerts regarding weak cryptographic algorithms when processing sensitive data. - Memoized `macAddress` in `local.ts` to avoid redundant system calls. - Upgraded `hashString` and `nonRandomUUID` to SHA-256 in `crypto.ts`. - Updated relevant unit tests to verify memoization and new hash lengths.
This PR improves Shopify CLI performance by memoizing the `macAddress` retrieval and modernizes cryptographic hashing to address CodeQL security alerts. - Memoized `macAddress` in `local.ts` to avoid redundant system calls. - Upgraded `hashString` and `nonRandomUUID` to SHA-256 in `crypto.ts`. - Ensured `nonRandomUUID` produces standard-length UUIDs by truncating the SHA-256 hash to 32 hex characters. - Updated all affected unit tests to reflect the new hash and UUID outputs.
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/crypto.d.ts@@ -20,10 +20,10 @@ export declare function base64URLEncode(str: Buffer): string;
*/
export declare function sha256(str: string): Buffer;
/**
- * Generate the SHA1 hash of a string.
+ * Generate the SHA256 hash of a string as a hex string.
*
* @param str - The string to hash.
- * @returns The SHA1 hash of the string.
+ * @returns The SHA256 hash of the string.
*/
export declare function hashString(str: string): string;
/**
|
1 participant
Goals
The goal is to improve the performance of the Shopify CLI by memoizing the
macAddressretrieval, which is used in every analytics event report.Changes
memoizedMacAddressto cache the result ofmacaddress.one().macaddressand assert that it is only called once even ifmacAddress()is invoked multiple times.How to test your changes?
Run the unit tests for the
cli-kitpackage:pnpm --filter @shopify/cli-kit vitest run src/public/node/context/local.test.tsAlso verify type-checking:
npx nx type-check cli-kitPR created automatically by Jules for task 4086787023136382130 started by @gonzaloriestra