perf: don't use await inside loops (#6617)

.changeset/kind-berries-wave.md

@@ -0,0 +1,17 @@
+---
+"@pnpm/plugin-commands-publishing": patch
+"@pnpm/plugin-commands-patching": patch
+"@pnpm/plugin-commands-listing": patch
+"@pnpm/resolve-dependencies": patch
+"@pnpm/exportable-manifest": patch
+"@pnpm/license-scanner": patch
+"@pnpm/get-context": patch
+"@pnpm/headless": patch
+"@pnpm/read-modules-dir": patch
+"@pnpm/package-store": patch
+"@pnpm/core": patch
+"@pnpm/audit": patch
+"@pnpm/list": patch
+---
+
+Don't use await in loops.

.changeset/stale-cups-compete.md

@@ -0,0 +1,5 @@
+---
+"pnpm": patch
+---
+
+Some minor performance improvements by removing await from loops [#6617](https://github.com/pnpm/pnpm/pull/6617).

__utils__/eslint-config/eslint.json

@@ -34,6 +34,7 @@
     "@typescript-eslint/explicit-function-return-type": "off",
     "@typescript-eslint/no-explicit-any": "error",
     "no-return-await": "error",
+    "no-await-in-loop": "error",
     "@typescript-eslint/return-await": "off",
     "@typescript-eslint/no-require-imports": "error",
     "@typescript-eslint/no-unused-expressions": "error",

cli/parse-cli-args/test/index.ts

@@ -244,6 +244,7 @@ test.each([
     // shouldn't affect its arg parsing. Test both scenarios for good measure.
     const input = [...(testWithCommandFallback ? [] : ['run']), ...testInput.split(' ')]
 
+    // eslint-disable-next-line no-await-in-loop
     const { options, cmd, params, fallbackCommandUsed } = await parseCliArgs({
       ...DEFAULT_OPTS,
       fallbackCommand: 'run',

env/plugin-commands-env/test/env.test.ts

@@ -119,6 +119,7 @@ test('it re-attempts failed downloads', async () => {
   try {
     const attempts = 2
     for (let i = 0; i < attempts; i++) {
+      // eslint-disable-next-line no-await-in-loop
       await expect(
         env.handler({
           bin: process.cwd(),

exec/lifecycle/src/runLifecycleHooksConcurrently.ts

@@ -52,7 +52,7 @@ export async function runLifecycleHooksConcurrently (
         let isBuilt = false
         for (const stage of (importerStages ?? stages)) {
           if ((manifest.scripts == null) || !manifest.scripts[stage]) continue
-          await runLifecycleHook(stage, manifest, runLifecycleHookOpts)
+          await runLifecycleHook(stage, manifest, runLifecycleHookOpts) // eslint-disable-line no-await-in-loop
           isBuilt = true
         }
         if (targetDirs == null || targetDirs.length === 0 || !isBuilt) return

exec/plugin-commands-rebuild/src/recursive.ts

@@ -111,6 +111,7 @@ export async function recursiveRebuild (
   }
   const limitRebuild = pLimit(opts.workspaceConcurrency ?? 4)
   for (const chunk of chunks) {
+    // eslint-disable-next-line no-await-in-loop
     await Promise.all(chunk.map(async (rootDir: string) =>
       limitRebuild(async () => {
         try {

exec/plugin-commands-script-runners/src/exec.ts

@@ -180,6 +180,7 @@ export async function handler (
 
   let exitCode = 0
   for (const chunk of chunks) {
+    // eslint-disable-next-line no-await-in-loop
     await Promise.all(chunk.map(async (prefix: string) =>
       limitRun(async () => {
         result[prefix].status = 'running'

exec/plugin-commands-script-runners/src/runRecursive.ts

@@ -90,6 +90,7 @@ export async function runRecursive (
       return specifiedScripts.map(script => ({ prefix, scriptName: script }))
     }).flat()
 
+    // eslint-disable-next-line no-await-in-loop
     await Promise.all(selectedScripts.map(async ({ prefix, scriptName }) =>
       limitRun(async () => {
         const pkg = opts.selectedProjectsGraph[prefix]

exec/prepare-package/src/index.ts

@@ -41,6 +41,7 @@ export async function preparePackage (opts: PreparePackageOptions, pkgDir: strin
     await runLifecycleHook(installScriptName, manifest, execOpts)
     for (const scriptName of PREPUBLISH_SCRIPTS) {
       if (manifest.scripts[scriptName] == null || manifest.scripts[scriptName] === '') continue
+      // eslint-disable-next-line no-await-in-loop
       await runLifecycleHook(scriptName, manifest, execOpts)
     }
   } catch (err: any) { // eslint-disable-line

fs/read-modules-dir/src/index.ts

@@ -17,21 +17,18 @@ async function _readModulesDir (
   modulesDir: string,
   scope?: string
 ) {
-  let pkgNames: string[] = []
+  const pkgNames: string[] = []
   const parentDir = scope ? path.join(modulesDir, scope) : modulesDir
-  for (const dir of await readdir(parentDir, { withFileTypes: true })) {
-    if (dir.isFile() || dir.name[0] === '.') continue
+  await Promise.all((await readdir(parentDir, { withFileTypes: true })).map(async (dir) => {
+    if (dir.isFile() || dir.name[0] === '.') return
 
     if (!scope && dir.name[0] === '@') {
-      pkgNames = [
-        ...pkgNames,
-        ...await _readModulesDir(modulesDir, dir.name),
-      ]
-      continue
+      pkgNames.push(...await _readModulesDir(modulesDir, dir.name))
+      return
     }
 
     const pkgName = scope ? `${scope}/${dir.name as string}` : dir.name
     pkgNames.push(pkgName)
-  }
+  }))
   return pkgNames
 }

lockfile/audit/src/index.ts

@@ -83,11 +83,12 @@ async function extendWithDependencyPaths (auditReport: AuditReport, opts: {
     include: opts.include,
   }
   const _searchPackagePaths = searchPackagePaths.bind(null, searchOpts, projectDirs)
-  for (const { findings, module_name: moduleName } of Object.values(advisories)) {
-    for (const finding of findings) {
-      finding.paths = await _searchPackagePaths(`${moduleName}@${finding.version}`)
-    }
-  }
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  await Promise.all(Object.values(advisories).map(async ({ findings, module_name }) => {
+    await Promise.all(findings.map(async (finding) => {
+      finding.paths = await _searchPackagePaths(`${module_name}@${finding.version}`)
+    }))
+  }))
   return auditReport
 }
 

lockfile/lockfile-file/src/read.ts

@@ -178,6 +178,7 @@ async function _readWantedLockfile (
     }
   }
   let result: { lockfile: Lockfile | null, hadConflicts: boolean } = { lockfile: null, hadConflicts: false }
+  /* eslint-disable no-await-in-loop */
   for (const lockfileName of lockfileNames) {
     result = await _read(path.join(pkgPath, lockfileName), pkgPath, { ...opts, autofixMergeConflicts: true })
     if (result.lockfile) {
@@ -187,6 +188,7 @@ async function _readWantedLockfile (
       break
     }
   }
+  /* eslint-enable no-await-in-loop */
   return result
 }
 

network/fetch/src/fetchFromRegistry.ts

@@ -48,6 +48,7 @@ export function createFetchFromRegistry (
     let redirects = 0
     let urlObject = new URL(url)
     const originalHost = urlObject.host
+    /* eslint-disable no-await-in-loop */
     while (true) {
       const agentOptions = {
         ...defaultOpts,
@@ -77,6 +78,7 @@ export function createFetchFromRegistry (
       if (!headers['authorization'] || originalHost === urlObject.host) continue
       delete headers.authorization
     }
+    /* eslint-enable no-await-in-loop */
   }
 }
 

patching/plugin-commands-patching/src/patchRemove.ts

@@ -57,13 +57,13 @@ export async function handler (opts: PatchRemoveCommandOptions, params: string[]
     throw new PnpmError('NO_PATCHES_TO_REMOVE', 'There are no patches that need to be removed')
   }
 
-  for (const patch of patchesToRemove) {
+  await Promise.all(patchesToRemove.map(async (patch) => {
     if (Object.prototype.hasOwnProperty.call(patchedDependencies, patch)) {
       const patchFile = path.join(lockfileDir, patchedDependencies[patch])
       await fs.rm(patchFile, { force: true })
       delete rootProjectManifest.pnpm!.patchedDependencies![patch]
     }
-  }
+  }))
 
   await writeProjectManifest(rootProjectManifest)
 

pkg-manager/core/src/install/allProjectsAreUpToDate.ts

@@ -74,44 +74,50 @@ async function linkedPackagesAreUpToDate (
     snapshot: ProjectSnapshot
   }
 ) {
-  for (const depField of DEPENDENCIES_FIELDS) {
-    const lockfileDeps = project.snapshot[depField]
-    const manifestDeps = project.manifest[depField]
-    if ((lockfileDeps == null) || (manifestDeps == null)) continue
-    const depNames = Object.keys(lockfileDeps)
-    for (const depName of depNames) {
-      const currentSpec = manifestDeps[depName]
-      if (!currentSpec) continue
-      const lockfileRef = lockfileDeps[depName]
-      const isLinked = lockfileRef.startsWith('link:')
-      if (
-        isLinked &&
-        (
-          currentSpec.startsWith('link:') ||
-          currentSpec.startsWith('file:') ||
-          currentSpec.startsWith('workspace:.')
-        )
-      ) {
-        continue
-      }
-      const linkedDir = isLinked
-        ? path.join(project.dir, lockfileRef.slice(5))
-        : workspacePackages?.[depName]?.[lockfileRef]?.dir
-      if (!linkedDir) continue
-      if (!linkWorkspacePackages && !currentSpec.startsWith('workspace:')) {
-        // we found a linked dir, but we don't want to use it, because it's not specified as a
-        // workspace:x.x.x dependency
-        continue
-      }
-      const linkedPkg = manifestsByDir[linkedDir] ?? await safeReadPackageJsonFromDir(linkedDir)
-      const availableRange = getVersionRange(currentSpec)
-      // This should pass the same options to semver as @pnpm/npm-resolver
-      const localPackageSatisfiesRange = availableRange === '*' || availableRange === '^' || availableRange === '~' ||
-        linkedPkg && semver.satisfies(linkedPkg.version, availableRange, { loose: true })
-      if (isLinked !== localPackageSatisfiesRange) return false
+  return pEvery(
+    DEPENDENCIES_FIELDS,
+    (depField) => {
+      const lockfileDeps = project.snapshot[depField]
+      const manifestDeps = project.manifest[depField]
+      if ((lockfileDeps == null) || (manifestDeps == null)) return true
+      const depNames = Object.keys(lockfileDeps)
+      return pEvery(
+        depNames,
+        async (depName) => {
+          const currentSpec = manifestDeps[depName]
+          if (!currentSpec) return true
+          const lockfileRef = lockfileDeps[depName]
+          const isLinked = lockfileRef.startsWith('link:')
+          if (
+            isLinked &&
+            (
+              currentSpec.startsWith('link:') ||
+              currentSpec.startsWith('file:') ||
+              currentSpec.startsWith('workspace:.')
+            )
+          ) {
+            return true
+          }
+          const linkedDir = isLinked
+            ? path.join(project.dir, lockfileRef.slice(5))
+            : workspacePackages?.[depName]?.[lockfileRef]?.dir
+          if (!linkedDir) return true
+          if (!linkWorkspacePackages && !currentSpec.startsWith('workspace:')) {
+            // we found a linked dir, but we don't want to use it, because it's not specified as a
+            // workspace:x.x.x dependency
+            return true
+          }
+          const linkedPkg = manifestsByDir[linkedDir] ?? await safeReadPackageJsonFromDir(linkedDir)
+          const availableRange = getVersionRange(currentSpec)
+          // This should pass the same options to semver as @pnpm/npm-resolver
+          const localPackageSatisfiesRange = availableRange === '*' || availableRange === '^' || availableRange === '~' ||
+            linkedPkg && semver.satisfies(linkedPkg.version, availableRange, { loose: true })
+          if (isLinked !== localPackageSatisfiesRange) return false
+          return true
+        }
+      )
     }
-  }
-  return true
+  )
 }
 
 function getVersionRange (spec: string) {

pkg-manager/core/src/install/index.ts

@@ -513,6 +513,7 @@ Note that in CI environments, this setting is enabled by default.`,
     let preferredSpecs: Record<string, string> | null = null
 
     // TODO: make it concurrent
+    /* eslint-disable no-await-in-loop */
     for (const project of projects) {
       const projectOpts = {
         ...project,
@@ -608,6 +609,7 @@ Note that in CI environments, this setting is enabled by default.`,
       }
       }
     }
+    /* eslint-enable no-await-in-loop */
 
     async function installCase (project: any) { // eslint-disable-line
       const wantedDependencies = getWantedDependencies(project.manifest, {

pkg-manager/core/src/link/index.ts

@@ -58,48 +58,49 @@ export async function link (
   }, true)
 
   const importerId = getLockfileImporterId(ctx.lockfileDir, opts.dir)
-  const linkedPkgs: Array<{ path: string, manifest: DependencyManifest, alias: string }> = []
   const specsToUpsert = [] as PackageSpecObject[]
 
-  for (const linkFrom of linkFromPkgs) {
-    let linkFromPath: string
-    let linkFromAlias: string | undefined
-    if (typeof linkFrom === 'string') {
-      linkFromPath = linkFrom
-    } else {
-      linkFromPath = linkFrom.path
-      linkFromAlias = linkFrom.alias
-    }
-    const { manifest } = await readProjectManifest(linkFromPath) as { manifest: DependencyManifest }
-    if (typeof linkFrom === 'string' && manifest.name === undefined) {
-      throw new PnpmError('INVALID_PACKAGE_NAME', `Package in ${linkFromPath} must have a name field to be linked`)
-    }
-
-    const targetDependencyType = getDependencyTypeFromManifest(opts.manifest, manifest.name) ?? opts.targetDependenciesField
-
-    specsToUpsert.push({
-      alias: manifest.name,
-      pref: getPref(manifest.name, manifest.name, manifest.version, {
-        pinnedVersion: opts.pinnedVersion,
-      }),
-      saveType: (targetDependencyType ?? (ctx.manifest && guessDependencyType(manifest.name, ctx.manifest))) as DependenciesField,
+  const linkedPkgs = await Promise.all(
+    linkFromPkgs.map(async (linkFrom) => {
+      let linkFromPath: string
+      let linkFromAlias: string | undefined
+      if (typeof linkFrom === 'string') {
+        linkFromPath = linkFrom
+      } else {
+        linkFromPath = linkFrom.path
+        linkFromAlias = linkFrom.alias
+      }
+      const { manifest } = await readProjectManifest(linkFromPath) as { manifest: DependencyManifest }
+      if (typeof linkFrom === 'string' && manifest.name === undefined) {
+        throw new PnpmError('INVALID_PACKAGE_NAME', `Package in ${linkFromPath} must have a name field to be linked`)
+      }
+
+      const targetDependencyType = getDependencyTypeFromManifest(opts.manifest, manifest.name) ?? opts.targetDependenciesField
+
+      specsToUpsert.push({
+        alias: manifest.name,
+        pref: getPref(manifest.name, manifest.name, manifest.version, {
+          pinnedVersion: opts.pinnedVersion,
+        }),
+        saveType: (targetDependencyType ?? (ctx.manifest && guessDependencyType(manifest.name, ctx.manifest))) as DependenciesField,
+      })
+
+      const packagePath = normalize(path.relative(opts.dir, linkFromPath))
+      const addLinkOpts = {
+        linkedPkgName: linkFromAlias ?? manifest.name,
+        manifest: ctx.manifest,
+        packagePath,
+      }
+      addLinkToLockfile(ctx.currentLockfile.importers[importerId], addLinkOpts)
+      addLinkToLockfile(ctx.wantedLockfile.importers[importerId], addLinkOpts)
+
+      return {
+        alias: linkFromAlias ?? manifest.name,
+        manifest,
+        path: linkFromPath,
+      }
     })
-
-    const packagePath = normalize(path.relative(opts.dir, linkFromPath))
-    const addLinkOpts = {
-      linkedPkgName: linkFromAlias ?? manifest.name,
-      manifest: ctx.manifest,
-      packagePath,
-    }
-    addLinkToLockfile(ctx.currentLockfile.importers[importerId], addLinkOpts)
-    addLinkToLockfile(ctx.wantedLockfile.importers[importerId], addLinkOpts)
-
-    linkedPkgs.push({
-      alias: linkFromAlias ?? manifest.name,
-      manifest,
-      path: linkFromPath,
-    })
-  }
+  )
 
   const updatedCurrentLockfile = pruneSharedLockfile(ctx.currentLockfile)
 
@@ -110,7 +111,7 @@ export async function link (
 
   // Linking should happen after removing orphans
   // Otherwise would've been removed
-  for (const { alias, manifest, path } of linkedPkgs) {
+  await Promise.all(linkedPkgs.map(async ({ alias, manifest, path }) => {
     // TODO: cover with test that linking reports with correct dependency types
     const stu = specsToUpsert.find((s) => s.alias === manifest.name)
     const targetDependencyType = getDependencyTypeFromManifest(opts.manifest, manifest.name) ?? opts.targetDependenciesField
@@ -119,7 +120,7 @@ export async function link (
       linkedPackage: manifest,
       prefix: opts.dir,
     })
-  }
+  }))
 
   const linkToBin = maybeOpts?.linkToBin ?? path.join(destModules, '.bin')
   await linkBinsOfPackages(linkedPkgs.map((p) => ({ manifest: p.manifest, location: p.path })), linkToBin, {