Review fixes

Author: brijeshb42

packages/code-infra/src/cli/cmdCopyFiles.mjs

@@ -2,7 +2,7 @@ import { findWorkspaceDir } from '@pnpm/find-workspace-dir';
 import { globby } from 'globby';
 import fs from 'node:fs/promises';
 import path from 'node:path';
-import { wrapInWorker } from '../utils/build.mjs';
+import { mapConcurrently } from '../utils/build.mjs';
 
 /**
  * @typedef {Object} Args
@@ -106,11 +106,12 @@ async function processGlobs({ globs, cwd, silent = true, buildDir }) {
     });
   });
 
-  await wrapInWorker(
+  await mapConcurrently(
+    filesToProcess,
     async (file) => {
       await recursiveCopy({ source: file.sourcePath, target: file.targetPath, silent });
     },
-    { items: filesToProcess },
+    50,
   );
   return filesToProcess.length;
 }

packages/code-infra/src/cli/cmdExtractErrorCodes.mjs

@@ -1,6 +1,6 @@
 /* eslint-disable no-console */
 
-import { measurePerf } from '../utils/build.mjs';
+import { markFn, measureFn } from '../utils/build.mjs';
 
 /**
  * @typedef {import('../utils/extractErrorCodes.mjs').Args} Args
@@ -29,11 +29,13 @@ export default /** @type {import('yargs').CommandModule<{}, Args>} */ ({
       });
   },
   async handler(args) {
-    console.log(`🔨 Extracting error codes`);
-    const duration = await measurePerf(/** @type {string} */ (args._[0]), async () => {
+    const commandName = /** @type {string} */ (args._[0]);
+    await markFn(commandName, async () => {
       const module = await import('../utils/extractErrorCodes.mjs');
       await module.default(args);
     });
-    console.log(`✅ Extracted error codes in ${(duration / 1000.0).toFixed(3)}s`);
+    console.log(
+      `✅ Extracted error codes in ${(measureFn(commandName).duration / 1000.0).toFixed(3)}s`,
+    );
   },
 });

packages/code-infra/src/cli/cmdJsonLint.mjs

@@ -4,7 +4,7 @@ import chalk from 'chalk';
 import fs from 'node:fs/promises';
 import { globby } from 'globby';
 import path from 'node:path';
-import { wrapInWorker } from '../utils/build.mjs';
+import { mapConcurrently } from '../utils/build.mjs';
 
 /**
  * @typedef {Object} Args
@@ -45,7 +45,8 @@ export default /** @type {import('yargs').CommandModule<{}, Args>} */ ({
 
     let passed = true;
 
-    await wrapInWorker(
+    await mapConcurrently(
+      filenames,
       async (filename) => {
         const content = await fs.readFile(path.join(cwd, filename), { encoding: 'utf8' });
         try {
@@ -59,7 +60,7 @@ export default /** @type {import('yargs').CommandModule<{}, Args>} */ ({
           console.error(failMessage(`Error parsing ${filename}:\n\n${String(error)}`));
         }
       },
-      { items: filenames, defaultConcurrency: 20, promiseMethod: 'allSettled' },
+      20,
     );
     if (!passed) {
       throw new Error('❌ At least one file did not pass. Check the console output');

packages/code-infra/src/utils/build.mjs

@@ -70,17 +70,31 @@ export function validatePkgJson(packageJson, options = {}) {
 }
 
 /**
- * Measures the performance of a function.
- * @param {string} label - The label for the performance measurement.
- * @param {Function} fn - The function to measure.
- * @returns {Promise<number>} - The duration of the function execution in milliseconds.
+ * Marks the start and end of a function execution for performance measurement.
+ * Uses the Performance API to create marks and measure the duration.
+ * @function
+ * @template {() => Promise<any>} F
+ * @param {string} label
+ * @param {() => ReturnType<F>} fn
+ * @returns {Promise<ReturnType<F>>}
+ */
+export async function markFn(label, fn) {
+  const startMark = `${label}-start`;
+  const endMark = `${label}-end`;
+  performance.mark(startMark);
+  const result = await fn();
+  performance.mark(endMark);
+  performance.measure(label, startMark, endMark);
+  return result;
+}
+
+/**
+ * @param {string} label
  */
-export async function measurePerf(label, fn) {
-  performance.mark(`${label}-start`);
-  await Promise.resolve(fn());
-  performance.mark(`${label}-end`);
-  const measurement = performance.measure(label, `${label}-start`, `${label}-end`);
-  return measurement.duration;
+export function measureFn(label) {
+  const startMark = `${label}-start`;
+  const endMark = `${label}-end`;
+  return performance.measure(label, startMark, endMark);
 }
 
 export const BASE_IGNORES = [
@@ -96,35 +110,40 @@ export const BASE_IGNORES = [
 ];
 
 /**
- * A utility to wrap a function in a worker pool.
+ * A utility to map a function over an array of items in a worker pool.
  *
  * This function will create a pool of workers and distribute the items to be processed among them.
  * Each worker will process items sequentially, but multiple workers will run in parallel.
+ *
  * @function
  * @template T
- * @param {(item: T) => Promise<void>} fn
- * @param {Object} options
- * @param {T[]} options.items
- * @param {number} [options.defaultConcurrency=50]
- * @param {'all' | 'allSettled'} [options.promiseMethod='all']
- * @returns {Promise<void>}
+ * @template R
+ * @param {T[]} items
+ * @param {(item: T) => Promise<R>} mapper
+ * @param {number} concurrency
+ * @returns {Promise<(R|Error)[]>}
  */
-export async function wrapInWorker(fn, options) {
-  const { defaultConcurrency = 50, items = [], promiseMethod = 'all' } = options ?? {};
-  if (items.length === 0) {
-    return;
+export async function mapConcurrently(items, mapper, concurrency) {
+  if (!items.length) {
+    return Promise.resolve([]); // nothing to do
   }
-  const itemIterator = items[Symbol.iterator]();
-  const concurrency = Math.min(defaultConcurrency, items.length);
+  const itemIterator = items.entries();
+  const count = Math.min(concurrency, items.length);
   const workers = [];
-  for (let i = 0; i < concurrency; i += 1) {
+  /**
+   * @type {(R|Error)[]}
+   */
+  const results = new Array(items.length);
+  for (let i = 0; i < count; i += 1) {
     const worker = Promise.resolve().then(async () => {
-      for (const item of itemIterator) {
+      for (const [index, item] of itemIterator) {
         // eslint-disable-next-line no-await-in-loop
-        await fn(item);
+        const res = await mapper(item);
+        results[index] = res;
       }
     });
     workers.push(worker);
   }
-  await (promiseMethod === 'all' ? Promise.all(workers) : Promise.allSettled(workers));
+  await Promise.all(workers);
+  return results;
 }

packages/code-infra/src/utils/extractErrorCodes.mjs

@@ -8,7 +8,7 @@ import * as fs from 'node:fs/promises';
 import * as path from 'node:path';
 
 import { getWorkspacePackages } from '../cli/pnpm.mjs';
-import { BASE_IGNORES, wrapInWorker } from './build.mjs';
+import { BASE_IGNORES, mapConcurrently } from './build.mjs';
 
 /**
  * @typedef {Object} Args
@@ -18,12 +18,12 @@ import { BASE_IGNORES, wrapInWorker } from './build.mjs';
  */
 
 /**
- * Extracts error codes from all files in a directory.
+ * Gets all relevant files for a package to parse.
+ *
  * @param {import('../cli/pnpm.mjs').PublicPackage} pkg
- * @param {Set<string>} errors
- * @param {import('@mui/internal-babel-plugin-minify-errors').Options['detection']} [detection='opt-in']
+ * @returns {Promise<string[]>} An array of file paths.
  */
-async function extractErrorCodesForPackage(pkg, errors, detection = 'opt-in') {
+async function getFilesForPackage(pkg) {
   const srcPath = path.join(pkg.path, 'src');
   const srcPathExists = await fs
     .stat(srcPath)
@@ -42,11 +42,19 @@ async function extractErrorCodesForPackage(pkg, errors, detection = 'opt-in') {
     ],
     cwd,
   });
-  const workerCount = Math.min(40, files.length);
-  console.log(`🔍 ${pkg.name}: Processing ${files.length} file${files.length > 1 ? 's' : ''}...`);
-  await wrapInWorker(
-    async (file) => {
-      const fullPath = path.join(cwd, file);
+  return files.map((file) => path.join(cwd, file));
+}
+
+/**
+ * Extracts error codes from all files in a directory.
+ * @param {string[]} files
+ * @param {Set<string>} errors
+ * @param {import('@mui/internal-babel-plugin-minify-errors').Options['detection']} [detection='opt-in']
+ */
+async function extractErrorCodesForWorkspace(files, errors, detection = 'opt-in') {
+  await mapConcurrently(
+    files,
+    async (fullPath) => {
       const code = await fs.readFile(fullPath, 'utf8');
       const ast = await parseAsync(code, {
         filename: fullPath,
@@ -73,7 +81,7 @@ async function extractErrorCodesForPackage(pkg, errors, detection = 'opt-in') {
         },
       });
     },
-    { defaultConcurrency: workerCount, items: files },
+    30,
   );
 }
 
@@ -86,6 +94,9 @@ export default async function extractErrorCodes(args) {
    * @type {Set<string>}
    */
   const errors = new Set();
+
+  // Find relevant files
+
   const basePackages = await getWorkspacePackages({
     publicOnly: true,
   });
@@ -97,7 +108,19 @@ export default async function extractErrorCodes(args) {
       !pkg.name.startsWith('@mui-internal/') &&
       !skipPackages.includes(pkg.name),
   );
-  await Promise.all(packages.map((pkg) => extractErrorCodesForPackage(pkg, errors, detection)));
+  const files = await Promise.all(packages.map((pkg) => getFilesForPackage(pkg)));
+  packages.forEach((pkg, index) => {
+    console.log(
+      `🔍 ${pkg.name}: Found ${files[index].length} file${files[index].length > 1 ? 's' : ''}`,
+    );
+  });
+
+  // Extract error codes from said files.
+  const filesToProcess = files.flat();
+  console.log(`🔍 Extracting error codes from ${filesToProcess.length} files...`);
+  await extractErrorCodesForWorkspace(filesToProcess, errors, detection);
+
+  // Write error codes to the specified file.
   const errorCodeFilePath = path.resolve(errorCodesPath);
   const fileExists = await fs
     .stat(errorCodeFilePath)
@@ -133,13 +156,13 @@ export default async function extractErrorCodes(args) {
   if (!fileExists) {
     await fs.mkdir(path.dirname(errorCodeFilePath), { recursive: true });
   }
-  await fs.writeFile(errorCodeFilePath, `${JSON.stringify(finalErrorCodes, null, 2)}\n`);
   const newErrorCount = inverseLookupCode.size - originalErrorCount;
   if (newErrorCount === 0) {
     console.log(`✅ No new error codes found.`);
   } else {
     console.log(
       `📝 Wrote ${newErrorCount} new error code${newErrorCount > 1 ? 's' : ''} to "${errorCodesPath}"`,
     );
+    await fs.writeFile(errorCodeFilePath, `${JSON.stringify(finalErrorCodes, null, 2)}\n`);
   }
 }