refactor: create a separate type for DepPath (#8094)

pnpm · May 20, 2024 · c649954 · c649954
1 parent 2b11645
commit c649954
Show file tree

Hide file tree

Showing 64 changed files with 734 additions and 577 deletions.
diff --git a/dedupe/check/src/dedupeDiffCheck.ts b/dedupe/check/src/dedupeDiffCheck.ts
@@ -4,7 +4,7 @@ import {
   type DedupeCheckIssues,
   type SnapshotsChanges,
 } from '@pnpm/dedupe.types'
-import { DEPENDENCIES_FIELDS } from '@pnpm/types'
+import { type DepPath, DEPENDENCIES_FIELDS } from '@pnpm/types'
 import { DedupeCheckIssuesError } from './DedupeCheckIssuesError'
 
 const PACKAGE_SNAPSHOT_DEP_FIELDS = ['dependencies', 'optionalDependencies'] as const
@@ -43,15 +43,15 @@ type KeyValueMatch<T, K, U> = K extends keyof T
 type PossiblyResolvedDependenciesKeys<TSnapshot> = KeysOfValue<TSnapshot, ResolvedDependencies | undefined>
 
 function diffSnapshots<TSnapshot> (
-  prev: Record<string, TSnapshot>,
-  next: Record<string, TSnapshot>,
+  prev: Record<DepPath, TSnapshot>,
+  next: Record<DepPath, TSnapshot>,
   fields: ReadonlyArray<PossiblyResolvedDependenciesKeys<TSnapshot>>
 ): SnapshotsChanges {
   const removed: string[] = []
   const updated: Record<string, ResolutionChangesByAlias> = {}
 
   for (const [id, prevSnapshot] of Object.entries(prev)) {
-    const nextSnapshot = next[id]
+    const nextSnapshot = next[id as DepPath]
 
     if (nextSnapshot == null) {
       removed.push(id)
@@ -68,7 +68,7 @@ function diffSnapshots<TSnapshot> (
     }
   }
 
-  const added = Object.keys(next).filter(id => prev[id] == null)
+  const added = (Object.keys(next) as DepPath[]).filter(id => prev[id] == null)
 
   return { added, removed, updated }
 }

diff --git a/dedupe/check/test/dedupeDiffCheck.ts b/dedupe/check/test/dedupeDiffCheck.ts
@@ -1,5 +1,6 @@
 import { DedupeCheckIssuesError, dedupeDiffCheck } from '@pnpm/dedupe.check'
 import { type Lockfile } from '@pnpm/lockfile-types'
+import { type DepPath } from '@pnpm/types'
 
 describe('dedupeDiffCheck', () => {
   it('should have no changes for same lockfile', () => {
@@ -38,15 +39,15 @@ describe('dedupeDiffCheck', () => {
         },
       },
       packages: {
-        '/is-positive@3.0.0': {
+        ['is-positive@3.0.0' as DepPath]: {
           resolution: {
             integrity: 'sha512-JDkaKp5jWv24ZaFuYDKTcBrC/wBOHdjhzLDkgrrkJD/j7KqqXsGcAkex336qHoOFEajMy7bYqUgm0KH9/MzQvw==',
           },
           engines: {
             node: '>=0.10.0',
           },
         },
-        '/is-positive@3.1.0': {
+        ['is-positive@3.1.0' as DepPath]: {
           resolution: {
             integrity: 'sha1-hX21hKG6XRyymAUn/DtsQ103sP0=',
           },
@@ -78,7 +79,7 @@ describe('dedupeDiffCheck', () => {
         },
       },
       packages: {
-        '/is-positive@3.1.0': {
+        ['is-positive@3.1.0' as DepPath]: {
           resolution: {
             integrity: 'sha1-hX21hKG6XRyymAUn/DtsQ103sP0=',
           },

diff --git a/deps/graph-builder/src/lockfileToDepGraph.ts b/deps/graph-builder/src/lockfileToDepGraph.ts
@@ -15,7 +15,7 @@ import {
 import { logger } from '@pnpm/logger'
 import { type IncludedDependencies } from '@pnpm/modules-yaml'
 import { packageIsInstallable } from '@pnpm/package-is-installable'
-import { type SupportedArchitectures, type PatchFile, type Registries } from '@pnpm/types'
+import { type DepPath, type SupportedArchitectures, type PatchFile, type Registries, type PkgIdWithPatchHash } from '@pnpm/types'
 import {
   type PkgRequestFetchResult,
   type FetchPackageToStoreFunction,
@@ -38,7 +38,8 @@ export interface DependenciesGraphNode {
   children: Record<string, string>
   optionalDependencies: Set<string>
   optional: boolean
-  depPath: string // this option is only needed for saving pendingBuild when running with --ignore-scripts flag
+  depPath: DepPath // this option is only needed for saving pendingBuild when running with --ignore-scripts flag
+  pkgIdWithPatchHash: PkgIdWithPatchHash
   isBuilt?: boolean
   requiresBuild?: boolean
   hasBin: boolean
@@ -63,7 +64,7 @@ export interface LockfileToDepGraphOptions {
   patchedDependencies?: Record<string, PatchFile>
   registries: Registries
   sideEffectsCacheRead: boolean
-  skipped: Set<string>
+  skipped: Set<DepPath>
   storeController: StoreController
   storeDir: string
   virtualStoreDir: string
@@ -100,12 +101,13 @@ export async function lockfileToDepGraph (
   if (lockfile.packages != null) {
     const pkgSnapshotByLocation: Record<string, PackageSnapshot> = {}
     await Promise.all(
-      Object.entries(lockfile.packages).map(async ([depPath, pkgSnapshot]) => {
+      (Object.entries(lockfile.packages) as Array<[DepPath, PackageSnapshot]>).map(async ([depPath, pkgSnapshot]) => {
         if (opts.skipped.has(depPath)) return
         // TODO: optimize. This info can be already returned by pkgSnapshotToResolution()
         const { name: pkgName, version: pkgVersion } = nameVerFromPkgSnapshot(depPath, pkgSnapshot)
         const modules = path.join(opts.virtualStoreDir, dp.depPathToFilename(depPath, opts.virtualStoreDirMaxLength), 'node_modules')
         const packageId = packageIdFromSnapshot(depPath, pkgSnapshot)
+        const pkgIdWithPatchHash = dp.getPkgIdWithPatchHash(depPath)
 
         const pkg = {
           name: pkgName,
@@ -183,6 +185,7 @@ export async function lockfileToDepGraph (
         }
         graph[dir] = {
           children: {},
+          pkgIdWithPatchHash,
           depPath,
           dir,
           fetching: fetchResponse.fetching,
@@ -242,7 +245,7 @@ function getChildrenPaths (
     virtualStoreDir: string
     storeDir: string
     skipped: Set<string>
-    pkgSnapshotsByDepPaths: Record<string, PackageSnapshot>
+    pkgSnapshotsByDepPaths: Record<DepPath, PackageSnapshot>
     lockfileDir: string
     sideEffectsCacheRead: boolean
     storeController: StoreController
@@ -259,7 +262,7 @@ function getChildrenPaths (
       children[alias] = path.resolve(ctx.lockfileDir, importerId, ref.slice(5))
       continue
     }
-    const childRelDepPath = dp.refToRelative(ref, alias) as string
+    const childRelDepPath = dp.refToRelative(ref, alias)!
     const childPkgSnapshot = ctx.pkgSnapshotsByDepPaths[childRelDepPath]
     if (ctx.graph[childRelDepPath]) {
       children[alias] = ctx.graph[childRelDepPath].dir

diff --git a/exec/build-modules/src/buildSequence.ts b/exec/build-modules/src/buildSequence.ts
@@ -1,10 +1,11 @@
 import { graphSequencer } from '@pnpm/deps.graph-sequencer'
-import { type PackageManifest, type PatchFile } from '@pnpm/types'
+import { type PkgIdWithPatchHash, type DepPath, type PackageManifest, type PatchFile } from '@pnpm/types'
 import filter from 'ramda/src/filter'
 
-export interface DependenciesGraphNode {
-  children: Record<string, string>
-  depPath: string
+export interface DependenciesGraphNode<T extends string> {
+  children: Record<string, T>
+  depPath: DepPath
+  pkgIdWithPatchHash: PkgIdWithPatchHash
   name: string
   dir: string
   fetchingBundledManifest?: () => Promise<PackageManifest | undefined>
@@ -20,14 +21,12 @@ export interface DependenciesGraphNode {
   patchFile?: PatchFile
 }
 
-export interface DependenciesGraph {
-  [depPath: string]: DependenciesGraphNode
-}
+export type DependenciesGraph<T extends string> = Record<T, DependenciesGraphNode<T>>
 
-export function buildSequence (
-  depGraph: Record<string, Pick<DependenciesGraphNode, 'children' | 'requiresBuild'>>,
+export function buildSequence<T extends string> (
+  depGraph: Record<string, Pick<DependenciesGraphNode<T>, 'children' | 'requiresBuild'>>,
   rootDepPaths: string[]
-): string[][] {
+): T[][] {
   const nodesToBuild = new Set<string>()
   getSubgraphToBuild(depGraph, rootDepPaths, nodesToBuild, new Set<string>())
   const onlyFromBuildGraph = filter((depPath: string) => nodesToBuild.has(depPath))
@@ -37,12 +36,12 @@ export function buildSequence (
       .map((depPath) => [depPath, onlyFromBuildGraph(Object.values(depGraph[depPath].children))])
   )
   const graphSequencerResult = graphSequencer(graph, nodesToBuildArray)
-  const chunks = graphSequencerResult.chunks as string[][]
+  const chunks = graphSequencerResult.chunks as T[][]
   return chunks
 }
 
-function getSubgraphToBuild (
-  graph: Record<string, Pick<DependenciesGraphNode, 'children' | 'requiresBuild' | 'patchFile'>>,
+function getSubgraphToBuild<T extends string> (
+  graph: Record<string, Pick<DependenciesGraphNode<T>, 'children' | 'requiresBuild' | 'patchFile'>>,
   entryNodes: string[],
   nodesToBuild: Set<string>,
   walked: Set<string>

diff --git a/exec/build-modules/src/index.ts b/exec/build-modules/src/index.ts
@@ -18,8 +18,8 @@ import { buildSequence, type DependenciesGraph, type DependenciesGraphNode } fro
 
 export type { DepsStateCache }
 
-export async function buildModules (
-  depGraph: DependenciesGraph,
+export async function buildModules<T extends string> (
+  depGraph: DependenciesGraph<T>,
   rootDepPaths: string[],
   opts: {
     allowBuild?: (pkgName: string) => boolean
@@ -55,7 +55,7 @@ export async function buildModules (
     builtHoistedDeps: opts.hoistedLocations ? {} : undefined,
     warn,
   }
-  const chunks = buildSequence(depGraph, rootDepPaths)
+  const chunks = buildSequence<T>(depGraph, rootDepPaths)
   const ignoredPkgs = new Set<string>()
   const allowBuild = opts.allowBuild
     ? (pkgName: string) => {
@@ -73,7 +73,7 @@ export async function buildModules (
       chunk = chunk.filter((depPath) => opts.depsToBuild!.has(depPath))
     }
 
-    return chunk.map((depPath: string) =>
+    return chunk.map((depPath) =>
       async () => {
         return buildDependency(depPath, depGraph, {
           ...buildDepOpts,
@@ -91,9 +91,9 @@ export async function buildModules (
   }
 }
 
-async function buildDependency (
-  depPath: string,
-  depGraph: DependenciesGraph,
+async function buildDependency<T extends string> (
+  depPath: T,
+  depGraph: DependenciesGraph<T>,
   opts: {
     extraBinPaths?: string[]
     extraNodePaths?: string[]
@@ -204,17 +204,17 @@ async function buildDependency (
   }
 }
 
-export async function linkBinsOfDependencies (
-  depNode: DependenciesGraphNode,
-  depGraph: DependenciesGraph,
+export async function linkBinsOfDependencies<T extends string> (
+  depNode: DependenciesGraphNode<T>,
+  depGraph: DependenciesGraph<T>,
   opts: {
     extraNodePaths?: string[]
     optional: boolean
     preferSymlinkedExecutables?: boolean
     warn: (message: string) => void
   }
 ): Promise<void> {
-  const childrenToLink: Record<string, string> = opts.optional
+  const childrenToLink: Record<string, T> = opts.optional
     ? depNode.children
     : pickBy((child, childAlias) => !depNode.optionalDependencies.has(childAlias), depNode.children)
 

diff --git a/exec/plugin-commands-rebuild/src/implementation/index.ts b/exec/plugin-commands-rebuild/src/implementation/index.ts
@@ -26,7 +26,7 @@ import { lockfileWalker, type LockfileWalkerStep } from '@pnpm/lockfile-walker'
 import { logger, streamParser } from '@pnpm/logger'
 import { writeModulesManifest } from '@pnpm/modules-yaml'
 import { createOrConnectStoreController } from '@pnpm/store-connection-manager'
-import { type ProjectManifest } from '@pnpm/types'
+import { type DepPath, type ProjectManifest } from '@pnpm/types'
 import { createAllowBuildFunction } from '@pnpm/builder.policy'
 import * as dp from '@pnpm/dependency-path'
 import { hardLinkDir } from '@pnpm/worker'
@@ -50,8 +50,8 @@ function findPackages (
   opts: {
     prefix: string
   }
-): string[] {
-  return Object.keys(packages)
+): DepPath[] {
+  return (Object.keys(packages) as DepPath[])
     .filter((relativeDepPath) => {
       const pkgLockfile = packages[relativeDepPath]
       const pkgInfo = nameVerFromPkgSnapshot(relativeDepPath, pkgLockfile)
@@ -206,7 +206,7 @@ export async function rebuildProjects (
 
 function getSubgraphToBuild (
   step: LockfileWalkerStep,
-  nodesToBuildAndTransitive: Set<string>,
+  nodesToBuildAndTransitive: Set<DepPath>,
   opts: {
     pkgsToRebuild: Set<string>
   }
@@ -254,7 +254,7 @@ async function _rebuild (
   const graph = new Map()
   const pkgSnapshots: PackageSnapshots = ctx.currentLockfile.packages ?? {}
 
-  const nodesToBuildAndTransitive = new Set<string>()
+  const nodesToBuildAndTransitive = new Set<DepPath>()
   getSubgraphToBuild(
     lockfileWalker(
       ctx.currentLockfile,
@@ -282,7 +282,7 @@ async function _rebuild (
     graph,
     nodesToBuildAndTransitiveArray
   )
-  const chunks = graphSequencerResult.chunks as string[][]
+  const chunks = graphSequencerResult.chunks as DepPath[][]
   const warn = (message: string) => {
     logger.info({ message, prefix: opts.dir })
   }
@@ -398,8 +398,8 @@ async function _rebuild (
   if (builtDepPaths.size > 0) {
     // It may be optimized because some bins were already linked before running lifecycle scripts
     await Promise.all(
-      Object
-        .keys(pkgSnapshots)
+      (Object
+        .keys(pkgSnapshots) as DepPath[])
         .filter((depPath) => !packageIsIndependent(pkgSnapshots[depPath]))
         .map(async (depPath) => limitLinking(async () => {
           const pkgSnapshot = pkgSnapshots[depPath]

diff --git a/lockfile/audit/test/index.ts b/lockfile/audit/test/index.ts
@@ -2,6 +2,7 @@ import { audit } from '@pnpm/audit'
 import { LOCKFILE_VERSION } from '@pnpm/constants'
 import { type PnpmError } from '@pnpm/error'
 import { fixtures } from '@pnpm/test-fixtures'
+import { type DepPath } from '@pnpm/types'
 import nock from 'nock'
 import { lockfileToAuditTree } from '../lib/lockfileToAuditTree'
 
@@ -22,12 +23,12 @@ describe('audit', () => {
       },
       lockfileVersion: LOCKFILE_VERSION,
       packages: {
-        'bar@1.0.0': {
+        ['bar@1.0.0' as DepPath]: {
           resolution: {
             integrity: 'bar-integrity',
           },
         },
-        'foo@1.0.0': {
+        ['foo@1.0.0' as DepPath]: {
           dependencies: {
             bar: '1.0.0',
           },
@@ -89,12 +90,12 @@ describe('audit', () => {
       },
       lockfileVersion: LOCKFILE_VERSION,
       packages: {
-        'bar@1.0.0': {
+        ['bar@1.0.0' as DepPath]: {
           resolution: {
             integrity: 'bar-integrity',
           },
         },
-        'foo@1.0.0': {
+        ['foo@1.0.0' as DepPath]: {
           dependencies: {
             bar: '1.0.0',
           },

diff --git a/lockfile/detect-dep-types/package.json b/lockfile/detect-dep-types/package.json
@@ -35,7 +35,8 @@
   },
   "dependencies": {
     "@pnpm/dependency-path": "workspace:*",
-    "@pnpm/lockfile-types": "workspace:*"
+    "@pnpm/lockfile-types": "workspace:*",
+    "@pnpm/types": "workspace:*"
   },
   "funding": "https://opencollective.com/pnpm",
   "exports": {

diff --git a/lockfile/detect-dep-types/src/index.ts b/lockfile/detect-dep-types/src/index.ts
@@ -1,5 +1,6 @@
 import { type Lockfile, type PackageSnapshots, type ResolvedDependencies } from '@pnpm/lockfile-types'
 import * as dp from '@pnpm/dependency-path'
+import { type DepPath } from '@pnpm/types'
 
 export enum DepType {
   DevOnly,
@@ -42,7 +43,7 @@ function detectDepTypesInSubGraph (
     walked: Set<string>
     dev: Record<string, DepType>
   },
-  depPaths: string[],
+  depPaths: DepPath[],
   opts: {
     dev: boolean
   }
@@ -70,8 +71,8 @@ function detectDepTypesInSubGraph (
   }
 }
 
-function resolvedDepsToDepPaths (deps: ResolvedDependencies): string[] {
+function resolvedDepsToDepPaths (deps: ResolvedDependencies): DepPath[] {
   return Object.entries(deps)
     .map(([alias, ref]) => dp.refToRelative(ref, alias))
-    .filter((depPath) => depPath !== null) as string[]
+    .filter((depPath) => depPath !== null) as DepPath[]
 }
diff --git a/lockfile/detect-dep-types/tsconfig.json b/lockfile/detect-dep-types/tsconfig.json
@@ -12,6 +12,9 @@
     {
       "path": "../../packages/dependency-path"
     },
+    {
+      "path": "../../packages/types"
+    },
     {
       "path": "../lockfile-types"
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -12,6 +12,9 @@ @@
         {
           "path": "../../packages/dependency-path"
         },
+        {
+          "path": "../../packages/types"
+        },
         {
           "path": "../lockfile-types"
         }
@@ Expand Down @@