perf: use depPath instead of pkgId to form a nodeId

.changeset/rotten-ligers-fold.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/resolve-dependencies": major
+---
+
+Use depPath in nodeIds instead of package IDs (depPath is unique as well but shorter).

.changeset/swift-dancers-dress.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/resolve-dependencies": major
+---
+
+`resolvedPackagesByPackageId` is replaced with `resolvedPackagesByDepPath`.

.changeset/unlucky-squids-count.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/package-requester": patch
+---
+
+The `finishing` promise is resolved always after the `files` promise.

packages/package-requester/src/packageRequester.ts

@@ -469,7 +469,6 @@ function fetchToStore (
           })
       )
       await writeJsonFile(filesIndexFile, { files: integrity })
-      finishing.resolve(undefined)
 
       if (isLocalTarballDep && opts.resolution['integrity']) { // eslint-disable-line @typescript-eslint/dot-notation
         await fs.mkdir(target, { recursive: true })
@@ -480,6 +479,7 @@ function fetchToStore (
         filesIndex: integrity,
         fromStore: false,
       })
+      finishing.resolve(undefined)
     } catch (err) {
       files.reject(err)
       if (opts.fetchRawManifest) {

packages/resolve-dependencies/src/index.ts

@@ -11,13 +11,13 @@ import {
   nodeIdContainsSequence,
 } from './nodeIdUtils'
 import resolveDependencies, {
-  ChildrenByParentId,
+  ChildrenByParentDepPath,
   DependenciesTree,
   LinkedDependency,
   PendingNode,
   PkgAddress,
   ResolvedPackage,
-  ResolvedPackagesByPackageId,
+  ResolvedPackagesByDepPath,
 } from './resolveDependencies'
 import R = require('ramda')
 
@@ -73,7 +73,7 @@ export default async function (
   const wantedToBeSkippedPackageIds = new Set<string>()
   const ctx = {
     alwaysTryWorkspacePackages: (opts.linkWorkspacePackagesDepth ?? -1) >= 0,
-    childrenByParentId: {} as ChildrenByParentId,
+    childrenByParentDepPath: {} as ChildrenByParentDepPath,
     currentLockfile: opts.currentLockfile,
     defaultTag: opts.tag,
     dependenciesTree: {} as DependenciesTree,
@@ -89,7 +89,7 @@ export default async function (
     pnpmVersion: opts.pnpmVersion,
     readPackageHook: opts.hooks.readPackage,
     registries: opts.registries,
-    resolvedPackagesByPackageId: {} as ResolvedPackagesByPackageId,
+    resolvedPackagesByDepPath: {} as ResolvedPackagesByDepPath,
     skipped: wantedToBeSkippedPackageIds,
     storeController: opts.storeController,
     updateMatching: opts.updateMatching,
@@ -117,7 +117,7 @@ export default async function (
       parentPkg: {
         installable: true,
         nodeId: `>${importer.id}>`,
-        pkgId: importer.id,
+        depPath: importer.id,
       },
       proceed,
       resolvedDependencies: {
@@ -139,7 +139,7 @@ export default async function (
   ctx.pendingNodes.forEach((pendingNode) => {
     ctx.dependenciesTree[pendingNode.nodeId] = {
       children: () => buildTree(ctx, pendingNode.nodeId, pendingNode.resolvedPackage.id,
-        ctx.childrenByParentId[pendingNode.resolvedPackage.id], pendingNode.depth + 1, pendingNode.installable),
+        ctx.childrenByParentDepPath[pendingNode.resolvedPackage.depPath], pendingNode.depth + 1, pendingNode.installable),
       depth: pendingNode.depth,
       installable: pendingNode.installable,
       resolvedPackage: pendingNode.resolvedPackage,
@@ -191,47 +191,47 @@ export default async function (
     dependenciesTree: ctx.dependenciesTree,
     outdatedDependencies: ctx.outdatedDependencies,
     resolvedImporters,
-    resolvedPackagesByPackageId: ctx.resolvedPackagesByPackageId,
+    resolvedPackagesByDepPath: ctx.resolvedPackagesByDepPath,
     wantedToBeSkippedPackageIds,
   }
 }
 
 function buildTree (
   ctx: {
-    childrenByParentId: ChildrenByParentId
+    childrenByParentDepPath: ChildrenByParentDepPath
     dependenciesTree: DependenciesTree
-    resolvedPackagesByPackageId: ResolvedPackagesByPackageId
+    resolvedPackagesByDepPath: ResolvedPackagesByDepPath
     skipped: Set<string>
   },
   parentNodeId: string,
   parentId: string,
-  children: Array<{alias: string, pkgId: string}>,
+  children: Array<{alias: string, depPath: string}>,
   depth: number,
   installable: boolean
 ) {
   const childrenNodeIds = {}
   for (const child of children) {
-    if (child.pkgId.startsWith('link:')) {
-      childrenNodeIds[child.alias] = child.pkgId
+    if (child.depPath.startsWith('link:')) {
+      childrenNodeIds[child.alias] = child.depPath
       continue
     }
-    if (nodeIdContainsSequence(parentNodeId, parentId, child.pkgId)) {
+    if (nodeIdContainsSequence(parentNodeId, parentId, child.depPath)) {
       continue
     }
-    const childNodeId = createNodeId(parentNodeId, child.pkgId)
+    const childNodeId = createNodeId(parentNodeId, child.depPath)
     childrenNodeIds[child.alias] = childNodeId
-    installable = installable && !ctx.skipped.has(child.pkgId)
+    installable = installable && !ctx.skipped.has(child.depPath)
     ctx.dependenciesTree[childNodeId] = {
       children: () => buildTree(ctx,
         childNodeId,
-        child.pkgId,
-        ctx.childrenByParentId[child.pkgId],
+        child.depPath,
+        ctx.childrenByParentDepPath[child.depPath],
         depth + 1,
         installable
       ),
       depth,
       installable,
-      resolvedPackage: ctx.resolvedPackagesByPackageId[child.pkgId],
+      resolvedPackage: ctx.resolvedPackagesByDepPath[child.depPath],
     }
   }
   return childrenNodeIds

packages/resolve-dependencies/src/resolveDependencies.ts

@@ -55,11 +55,11 @@ const dependencyResolvedLogger = logger('_dependency_resolved')
 
 export function nodeIdToParents (
   nodeId: string,
-  resolvedPackagesByPackageId: ResolvedPackagesByPackageId
+  resolvedPackagesByDepPath: ResolvedPackagesByDepPath
 ) {
   return splitNodeId(nodeId).slice(1)
-    .map((pkgId) => {
-      const { id, name, version } = resolvedPackagesByPackageId[pkgId]
+    .map((depPath) => {
+      const { id, name, version } = resolvedPackagesByDepPath[depPath]
       return { id, name, version }
     })
 }
@@ -87,13 +87,12 @@ export interface DependenciesTree {
   [nodeId: string]: DependenciesTreeNode
 }
 
-export interface ResolvedPackagesByPackageId {
-  [packageId: string]: ResolvedPackage
-}
+export type ResolvedPackagesByDepPath = Record<string, ResolvedPackage>
 
 export interface LinkedDependency {
   isLinkedDependency: true
   optional: boolean
+  depPath: string
   dev: boolean
   resolution: DirectoryResolution
   pkgId: string
@@ -111,10 +110,10 @@ export interface PendingNode {
   installable: boolean
 }
 
-export interface ChildrenByParentId {
-  [parentId: string]: Array<{
+export interface ChildrenByParentDepPath {
+  [depPath: string]: Array<{
     alias: string
-    pkgId: string
+    depPath: string
   }>
 }
 
@@ -123,9 +122,9 @@ export interface ResolutionContext {
   defaultTag: string
   dryRun: boolean
   forceFullResolution: boolean
-  resolvedPackagesByPackageId: ResolvedPackagesByPackageId
+  resolvedPackagesByDepPath: ResolvedPackagesByDepPath
   outdatedDependencies: {[pkgId: string]: string}
-  childrenByParentId: ChildrenByParentId
+  childrenByParentDepPath: ChildrenByParentDepPath
   pendingNodes: PendingNode[]
   wantedLockfile: Lockfile
   currentLockfile: Lockfile
@@ -150,6 +149,7 @@ export interface ResolutionContext {
 export type PkgAddress = {
   alias: string
   depIsLinked: boolean
+  depPath: string
   isNew: boolean
   isLinkedDependency?: false
   nodeId: string
@@ -201,7 +201,7 @@ export interface ResolvedPackage {
   }
 }
 
-type ParentPkg = Pick<PkgAddress, 'nodeId' | 'installable' | 'pkgId'>
+type ParentPkg = Pick<PkgAddress, 'nodeId' | 'installable' | 'depPath'>
 
 export default async function resolveDependencies (
   ctx: ResolutionContext,
@@ -271,7 +271,7 @@ export default async function resolveDependencies (
           if (!resolveDependencyResult.isNew) return resolveDependencyResult
 
           const resolveChildren = async function (preferredVersions: PreferredVersions) {
-            const resolvedPackage = ctx.resolvedPackagesByPackageId[resolveDependencyResult.pkgId]
+            const resolvedPackage = ctx.resolvedPackagesByDepPath[resolveDependencyResult.depPath]
             const currentResolvedDependencies = extendedWantedDep.infoFromLockfile?.dependencyLockfile ? {
               ...extendedWantedDep.infoFromLockfile.dependencyLockfile.dependencies,
               ...extendedWantedDep.infoFromLockfile.dependencyLockfile.optionalDependencies,
@@ -305,9 +305,9 @@ export default async function resolveDependencies (
                 workspacePackages,
               }
             ) as PkgAddress[]
-            ctx.childrenByParentId[resolveDependencyResult.pkgId] = children.map((child) => ({
+            ctx.childrenByParentDepPath[resolveDependencyResult.depPath] = children.map((child) => ({
               alias: child.alias,
-              pkgId: child.pkgId,
+              depPath: child.depPath,
             }))
             ctx.dependenciesTree[resolveDependencyResult.nodeId] = {
               children: children.reduce((chn, child) => {
@@ -329,8 +329,8 @@ export default async function resolveDependencies (
     .filter(Boolean) as PkgAddress[]
 
   const newPreferredVersions = { ...preferredVersions }
-  for (const { pkgId } of pkgAddresses) {
-    const resolvedPackage = ctx.resolvedPackagesByPackageId[pkgId]
+  for (const { depPath } of pkgAddresses) {
+    const resolvedPackage = ctx.resolvedPackagesByDepPath[depPath]
     if (!resolvedPackage) continue // This will happen only with linked dependencies
     if (!newPreferredVersions[resolvedPackage.name]) {
       newPreferredVersions[resolvedPackage.name] = {}
@@ -551,20 +551,20 @@ async function resolveDependency (
           pref: wantedDependency.pref,
           version: wantedDependency.alias ? wantedDependency.pref : undefined,
         },
-        parents: nodeIdToParents(options.parentPkg.nodeId, ctx.resolvedPackagesByPackageId),
+        parents: nodeIdToParents(options.parentPkg.nodeId, ctx.resolvedPackagesByDepPath),
         prefix: ctx.prefix,
         reason: 'resolution_failure',
       })
       return null
     }
-    err.pkgsStack = nodeIdToParents(options.parentPkg.nodeId, ctx.resolvedPackagesByPackageId)
+    err.pkgsStack = nodeIdToParents(options.parentPkg.nodeId, ctx.resolvedPackagesByDepPath)
     throw err
   }
 
   dependencyResolvedLogger.debug({
     resolution: pkgResponse.body.id,
     wanted: {
-      dependentId: options.parentPkg.pkgId,
+      dependentId: options.parentPkg.depPath,
       name: wantedDependency.alias,
       rawSpec: wantedDependency.pref,
     },
@@ -584,6 +584,7 @@ async function resolveDependency (
     const manifest = pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!() // eslint-disable-line @typescript-eslint/dot-notation
     return {
       alias: wantedDependency.alias || manifest.name,
+      depPath: pkgResponse.body.id,
       dev: wantedDependency.dev,
       isLinkedDependency: true,
       name: manifest.name,
@@ -595,6 +596,18 @@ async function resolveDependency (
     }
   }
 
+  let pkg: PackageManifest
+  let useManifestInfoFromLockfile = false
+  let prepare!: boolean
+  let hasBin!: boolean
+  pkg = ctx.readPackageHook
+    ? ctx.readPackageHook(pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!())
+    : pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!()
+  if (!pkg.name) { // TODO: don't fail on optional dependencies
+    throw new PnpmError('MISSING_PACKAGE_NAME', `Can't install ${wantedDependency.pref}: Missing package name`)
+  }
+  const depPath = dp.relative(ctx.registries, pkg.name, pkgResponse.body.id)
+
   // We are building the dependency tree only until there are new packages
   // or the packages repeat in a unique order.
   // This is needed later during peer dependencies resolution.
@@ -613,21 +626,13 @@ async function resolveDependency (
   if (
     nodeIdContainsSequence(
       options.parentPkg.nodeId,
-      options.parentPkg.pkgId,
-      pkgResponse.body.id
+      options.parentPkg.depPath,
+      depPath
     )
   ) {
     return null
   }
 
-  let pkg: PackageManifest
-  let useManifestInfoFromLockfile = false
-  let prepare!: boolean
-  let hasBin!: boolean
-  pkg = ctx.readPackageHook
-    ? ctx.readPackageHook(pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!())
-    : pkgResponse.body.manifest ?? await pkgResponse.bundledManifest!()
-
   if (
     !options.update && currentPkg.dependencyLockfile && currentPkg.depPath &&
     !pkgResponse.body.updated &&
@@ -660,9 +665,6 @@ async function resolveDependency (
     hasBin = Boolean((pkg.bin && !R.isEmpty(pkg.bin)) ?? pkg.directories?.bin)
     /* eslint-enable @typescript-eslint/dot-notation */
   }
-  if (!pkg.name) { // TODO: don't fail on optional dependencies
-    throw new PnpmError('MISSING_PACKAGE_NAME', `Can't install ${wantedDependency.pref}: Missing package name`)
-  }
   if (options.currentDepth === 0 && pkgResponse.body.latest && pkgResponse.body.latest !== pkg.version) {
     ctx.outdatedDependencies[pkgResponse.body.id] = pkgResponse.body.latest
   }
@@ -681,7 +683,7 @@ async function resolveDependency (
   // we only ever need to analyze one leaf dep in a graph, so the nodeId can be short and stateless.
   const nodeId = pkgIsLeaf(pkg)
     ? pkgResponse.body.id
-    : createNodeId(options.parentPkg.nodeId, pkgResponse.body.id)
+    : createNodeId(options.parentPkg.nodeId, depPath)
 
   const currentIsInstallable = (
     ctx.force ||
@@ -694,7 +696,7 @@ async function resolveDependency (
       })
   )
   const installable = parentIsInstallable && currentIsInstallable !== false
-  const isNew = !ctx.resolvedPackagesByPackageId[pkgResponse.body.id]
+  const isNew = !ctx.resolvedPackagesByDepPath[depPath]
 
   if (isNew) {
     if (currentIsInstallable !== true || !parentIsInstallable) {
@@ -720,9 +722,9 @@ async function resolveDependency (
         })
     }
 
-    ctx.resolvedPackagesByPackageId[pkgResponse.body.id] = getResolvedPackage({
+    ctx.resolvedPackagesByDepPath[depPath] = getResolvedPackage({
       dependencyLockfile: currentPkg.dependencyLockfile,
-      depPath: dp.relative(ctx.registries, pkg.name, pkgResponse.body.id),
+      depPath,
       force: ctx.force,
       hasBin,
       pkg,
@@ -731,9 +733,9 @@ async function resolveDependency (
       wantedDependency,
     })
   } else {
-    ctx.resolvedPackagesByPackageId[pkgResponse.body.id].prod = ctx.resolvedPackagesByPackageId[pkgResponse.body.id].prod || !wantedDependency.dev && !wantedDependency.optional
-    ctx.resolvedPackagesByPackageId[pkgResponse.body.id].dev = ctx.resolvedPackagesByPackageId[pkgResponse.body.id].dev || wantedDependency.dev
-    ctx.resolvedPackagesByPackageId[pkgResponse.body.id].optional = ctx.resolvedPackagesByPackageId[pkgResponse.body.id].optional && wantedDependency.optional
+    ctx.resolvedPackagesByDepPath[depPath].prod = ctx.resolvedPackagesByDepPath[depPath].prod || !wantedDependency.dev && !wantedDependency.optional
+    ctx.resolvedPackagesByDepPath[depPath].dev = ctx.resolvedPackagesByDepPath[depPath].dev || wantedDependency.dev
+    ctx.resolvedPackagesByDepPath[depPath].optional = ctx.resolvedPackagesByDepPath[depPath].optional && wantedDependency.optional
 
     if (ctx.dependenciesTree[nodeId]) {
       ctx.dependenciesTree[nodeId].depth = Math.min(ctx.dependenciesTree[nodeId].depth, options.currentDepth)
@@ -743,7 +745,7 @@ async function resolveDependency (
         depth: options.currentDepth,
         installable,
         nodeId,
-        resolvedPackage: ctx.resolvedPackagesByPackageId[pkgResponse.body.id],
+        resolvedPackage: ctx.resolvedPackagesByDepPath[depPath],
       })
     }
   }
@@ -751,6 +753,7 @@ async function resolveDependency (
   return {
     alias: wantedDependency.alias || pkg.name,
     depIsLinked,
+    depPath,
     isNew,
     nodeId,
     normalizedPref: options.currentDepth === 0 ? pkgResponse.body.normalizedPref : undefined,

packages/supi/src/install/index.ts

@@ -30,7 +30,6 @@ import { safeReadPackageFromDir as safeReadPkgFromDir } from '@pnpm/read-package
 import { removeBin } from '@pnpm/remove-bins'
 import resolveDependencies, {
   ResolvedDirectDependency,
-  ResolvedPackage,
 } from '@pnpm/resolve-dependencies'
 import {
   PreferredVersions,
@@ -610,7 +609,7 @@ async function installInContext (
     dependenciesTree,
     outdatedDependencies,
     resolvedImporters,
-    resolvedPackagesByPackageId,
+    resolvedPackagesByDepPath,
     wantedToBeSkippedPackageIds,
   } = await resolveDependencies(
     projectsToResolve,
@@ -780,17 +779,8 @@ async function installInContext (
     }))
   }
 
-  // waiting till the skipped packages are downloaded to the store
-  await Promise.all(
-    R.props<string, ResolvedPackage>(Array.from(wantedToBeSkippedPackageIds), resolvedPackagesByPackageId)
-      // skipped packages might have not been reanalized on a repeat install
-      // so lets just ignore those by excluding nulls
-      .filter(Boolean)
-      .map(({ fetchingFiles }) => fetchingFiles())
-  )
-
   // waiting till package requests are finished
-  await Promise.all(R.values(resolvedPackagesByPackageId).map(({ finishing }) => finishing()))
+  await Promise.all(R.values(resolvedPackagesByDepPath).map(({ finishing }) => finishing()))
 
   const lockfileOpts = { forceSharedFormat: opts.forceSharedLockfile }
   if (opts.lockfileOnly) {