fix: prefer headless installation when aliases are used

PR #2510

.changeset/angry-weeks-collect.md

@@ -0,0 +1,5 @@
+---
+"supi": patch
+---
+
+Headless installation should be preferred when local dependencies that use aliases are up-to-date.

packages/supi/src/install/allProjectsAreUpToDate.ts

@@ -0,0 +1,107 @@
+import { ProjectOptions } from '@pnpm/get-context'
+import {
+  Lockfile,
+  ProjectSnapshot,
+} from '@pnpm/lockfile-file'
+import { satisfiesPackageManifest } from '@pnpm/lockfile-utils'
+import { safeReadPackageFromDir as safeReadPkgFromDir } from '@pnpm/read-package-json'
+import { WorkspacePackages } from '@pnpm/resolver-base'
+import {
+  DEPENDENCIES_FIELDS,
+  DependencyManifest,
+  ProjectManifest,
+} from '@pnpm/types'
+import pEvery from 'p-every'
+import path = require('path')
+import R = require('ramda')
+import semver = require('semver')
+
+export default async function allProjectsAreUpToDate (
+  projects: Array<ProjectOptions & { id: string }>,
+  opts: {
+    wantedLockfile: Lockfile,
+    workspacePackages: WorkspacePackages,
+  },
+) {
+  const manifestsByDir = opts.workspacePackages ? getWorkspacePackagesByDirectory(opts.workspacePackages) : {}
+  const _satisfiesPackageManifest = satisfiesPackageManifest.bind(null, opts.wantedLockfile)
+  const _linkedPackagesAreUpToDate = linkedPackagesAreUpToDate.bind(null, manifestsByDir, opts.workspacePackages)
+  return pEvery(projects, async (project) => {
+    const importer = opts.wantedLockfile.importers[project.id]
+    return importer && !hasLocalTarballDepsInRoot(importer) &&
+      _satisfiesPackageManifest(project.manifest, project.id) &&
+      _linkedPackagesAreUpToDate(project.manifest, importer, project.rootDir)
+  })
+}
+
+function getWorkspacePackagesByDirectory (workspacePackages: WorkspacePackages) {
+  const workspacePackagesByDirectory = {}
+  Object.keys(workspacePackages || {}).forEach((pkgName) => {
+    Object.keys(workspacePackages[pkgName] || {}).forEach((pkgVersion) => {
+      workspacePackagesByDirectory[workspacePackages[pkgName][pkgVersion].dir] = workspacePackages[pkgName][pkgVersion].manifest
+    })
+  })
+  return workspacePackagesByDirectory
+}
+
+async function linkedPackagesAreUpToDate (
+  manifestsByDir: Record<string, DependencyManifest>,
+  workspacePackages: WorkspacePackages,
+  manifest: ProjectManifest,
+  projectSnapshot: ProjectSnapshot,
+  projectDir: string,
+) {
+  for (const depField of DEPENDENCIES_FIELDS) {
+    const lockfileDeps = projectSnapshot[depField]
+    const manifestDeps = manifest[depField]
+    if (!lockfileDeps || !manifestDeps) 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:')
+        )
+      ) {
+        continue
+      }
+      const linkedDir = isLinked
+        ? path.join(projectDir, lockfileRef.substr(5))
+        : workspacePackages?.[depName]?.[lockfileRef]?.dir
+      if (!linkedDir) continue
+      const linkedPkg = manifestsByDir[linkedDir] ?? await safeReadPkgFromDir(linkedDir)
+      const availableRange = getVersionRange(currentSpec)
+      // This should pass the same options to semver as @pnpm/npm-resolver
+      const localPackageSatisfiesRange = availableRange === '*' ||
+        linkedPkg && semver.satisfies(linkedPkg.version, availableRange, { loose: true })
+      if (isLinked !== localPackageSatisfiesRange) return false
+    }
+  }
+  return true
+}
+
+function getVersionRange (spec: string) {
+  if (spec.startsWith('workspace:')) return spec.substr(10)
+  if (spec.startsWith('npm:')) {
+    spec = spec.substr(4)
+    const index = spec.indexOf('@', 1)
+    if (index === -1) return '*'
+    return spec.substr(index + 1) || '*'
+  }
+  return spec
+}
+
+function hasLocalTarballDepsInRoot (importer: ProjectSnapshot) {
+  return R.any(refIsLocalTarball, Object.values(importer.dependencies ?? {}))
+    || R.any(refIsLocalTarball, Object.values(importer.devDependencies ?? {}))
+    || R.any(refIsLocalTarball, Object.values(importer.optionalDependencies ?? {}))
+}
+
+function refIsLocalTarball (ref: string) {
+  return ref.startsWith('file:') && (ref.endsWith('.tgz') || ref.endsWith('.tar.gz') || ref.endsWith('.tar'))
+}

packages/supi/src/install/index.ts

@@ -17,13 +17,11 @@ import {
 } from '@pnpm/lifecycle'
 import linkBins from '@pnpm/link-bins'
 import {
-  Lockfile,
   ProjectSnapshot,
   writeCurrentLockfile,
   writeLockfiles,
   writeWantedLockfile,
 } from '@pnpm/lockfile-file'
-import { satisfiesPackageManifest } from '@pnpm/lockfile-utils'
 import logger, { streamParser } from '@pnpm/logger'
 import { getAllDependenciesFromManifest } from '@pnpm/manifest-utils'
 import { write as writeModulesYaml } from '@pnpm/modules-yaml'
@@ -48,18 +46,17 @@ import rimraf = require('@zkochan/rimraf')
 import * as dp from 'dependency-path'
 import isInnerLink = require('is-inner-link')
 import isSubdir = require('is-subdir')
-import pEvery from 'p-every'
 import pFilter = require('p-filter')
 import pLimit from 'p-limit'
 import path = require('path')
 import R = require('ramda')
-import semver = require('semver')
 import getSpecFromPackageManifest from '../getSpecFromPackageManifest'
 import lock from '../lock'
 import parseWantedDependencies from '../parseWantedDependencies'
 import safeIsInnerLink from '../safeIsInnerLink'
 import removeDeps from '../uninstall/removeDeps'
 import { updateProjectManifest } from '../utils/getPref'
+import allProjectsAreUpToDate from './allProjectsAreUpToDate'
 import extendOptions, {
   InstallOptions,
   StrictInstallOptions,
@@ -190,11 +187,7 @@ export async function mutateModules (
         (!opts.pruneLockfileImporters || Object.keys(ctx.wantedLockfile.importers).length === ctx.projects.length) &&
         ctx.existsWantedLockfile &&
         ctx.wantedLockfile.lockfileVersion === LOCKFILE_VERSION &&
-        await pEvery(ctx.projects, async (project) =>
-          !hasLocalTarballDepsInRoot(ctx.wantedLockfile, project.id) &&
-          satisfiesPackageManifest(ctx.wantedLockfile, project.manifest, project.id) &&
-          linkedPackagesAreUpToDate(project.manifest, ctx.wantedLockfile.importers[project.id], project.rootDir, opts.workspacePackages),
-        )
+        await allProjectsAreUpToDate(ctx.projects, { wantedLockfile: ctx.wantedLockfile, workspacePackages: opts.workspacePackages })
       )
     ) {
       if (!ctx.existsWantedLockfile) {
@@ -523,59 +516,6 @@ function forgetResolutionsOfPrevWantedDeps (importer: ProjectSnapshot, wantedDep
   }
 }
 
-async function linkedPackagesAreUpToDate (
-  manifest: ProjectManifest,
-  projectSnapshot: ProjectSnapshot,
-  prefix: string,
-  workspacePackages?: WorkspacePackages,
-) {
-  const workspacePackagesByDirectory = workspacePackages ? getWorkspacePackagesByDirectory(workspacePackages) : {}
-  for (const depField of DEPENDENCIES_FIELDS) {
-    const importerDeps = projectSnapshot[depField]
-    const pkgDeps = manifest[depField]
-    if (!importerDeps || !pkgDeps) continue
-    const depNames = Object.keys(importerDeps)
-    for (const depName of depNames) {
-      if (!pkgDeps[depName]) continue
-      const isLinked = importerDeps[depName].startsWith('link:')
-      if (isLinked && (pkgDeps[depName].startsWith('link:') || pkgDeps[depName].startsWith('file:'))) continue
-      const dir = isLinked
-        ? path.join(prefix, importerDeps[depName].substr(5))
-        : workspacePackages?.[depName]?.[importerDeps[depName]]?.dir
-      if (!dir) continue
-      const linkedPkg = workspacePackagesByDirectory[dir] || await safeReadPkgFromDir(dir)
-      const availableRange = pkgDeps[depName].startsWith('workspace:') ? pkgDeps[depName].substr(10) : pkgDeps[depName]
-      // This should pass the same options to semver as @pnpm/npm-resolver
-      const localPackageSatisfiesRange = availableRange === '*' ||
-        linkedPkg && semver.satisfies(linkedPkg.version, availableRange, { loose: true })
-      if (isLinked !== localPackageSatisfiesRange) return false
-    }
-  }
-  return true
-}
-
-function getWorkspacePackagesByDirectory (workspacePackages: WorkspacePackages) {
-  const workspacePackagesByDirectory = {}
-  Object.keys(workspacePackages || {}).forEach((pkgName) => {
-    Object.keys(workspacePackages[pkgName] || {}).forEach((pkgVersion) => {
-      workspacePackagesByDirectory[workspacePackages[pkgName][pkgVersion].dir] = workspacePackages[pkgName][pkgVersion].manifest
-    })
-  })
-  return workspacePackagesByDirectory
-}
-
-function hasLocalTarballDepsInRoot (lockfile: Lockfile, importerId: string) {
-  const importer = lockfile.importers?.[importerId]
-  if (!importer) return false
-  return R.any(refIsLocalTarball, R.values(importer.dependencies || {}))
-    || R.any(refIsLocalTarball, R.values(importer.devDependencies || {}))
-    || R.any(refIsLocalTarball, R.values(importer.optionalDependencies || {}))
-}
-
-function refIsLocalTarball (ref: string) {
-  return ref.startsWith('file:') && (ref.endsWith('.tgz') || ref.endsWith('.tar.gz') || ref.endsWith('.tar'))
-}
-
 export async function addDependenciesToPackage (
   manifest: ProjectManifest,
   dependencySelectors: string[],

packages/supi/test/allProjectsAreUpToDate.test.ts

@@ -0,0 +1,129 @@
+import allProjectsAreUpToDate from 'supi/lib/install/allProjectsAreUpToDate'
+import tape = require('tape')
+import promisifyTape from 'tape-promise'
+
+const test = promisifyTape(tape)
+
+const fooManifest = {
+  name: 'foo',
+  version: '1.0.0',
+}
+const workspacePackages = {
+  foo: {
+    '1.0.0': {
+      dir: 'foo',
+      manifest: fooManifest,
+    },
+  },
+}
+
+test('allProjectsAreUpToDate(): works with aliased local dependencies', async (t: tape.Test) => {
+  t.ok(await allProjectsAreUpToDate([
+    {
+      id: 'bar',
+      manifest: {
+        dependencies: {
+          alias: 'npm:foo',
+        },
+      },
+      rootDir: 'bar',
+    },
+    {
+      id: 'foo',
+      manifest: fooManifest,
+      rootDir: 'foo',
+    },
+  ], {
+    wantedLockfile: {
+      importers: {
+        bar: {
+          dependencies: {
+            alias: 'link:../foo',
+          },
+          specifiers: {
+            alias: 'npm:foo',
+          },
+        },
+        foo: {
+          specifiers: {},
+        },
+      },
+      lockfileVersion: 5,
+    },
+    workspacePackages,
+  }))
+})
+
+test('allProjectsAreUpToDate(): works with aliased local dependencies that specify versions', async (t: tape.Test) => {
+  t.ok(await allProjectsAreUpToDate([
+    {
+      id: 'bar',
+      manifest: {
+        dependencies: {
+          alias: 'npm:foo@1',
+        },
+      },
+      rootDir: 'bar',
+    },
+    {
+      id: 'foo',
+      manifest: fooManifest,
+      rootDir: 'foo',
+    },
+  ], {
+    wantedLockfile: {
+      importers: {
+        bar: {
+          dependencies: {
+            alias: 'link:../foo',
+          },
+          specifiers: {
+            alias: 'npm:foo@1',
+          },
+        },
+        foo: {
+          specifiers: {},
+        },
+      },
+      lockfileVersion: 5,
+    },
+    workspacePackages,
+  }))
+})
+
+test('allProjectsAreUpToDate(): returns false if the aliased dependency version is out of date', async (t: tape.Test) => {
+  t.notOk(await allProjectsAreUpToDate([
+    {
+      id: 'bar',
+      manifest: {
+        dependencies: {
+          alias: 'npm:foo@0',
+        },
+      },
+      rootDir: 'bar',
+    },
+    {
+      id: 'foo',
+      manifest: fooManifest,
+      rootDir: 'foo',
+    },
+  ], {
+    wantedLockfile: {
+      importers: {
+        bar: {
+          dependencies: {
+            alias: 'link:../foo',
+          },
+          specifiers: {
+            alias: 'npm:foo@0',
+          },
+        },
+        foo: {
+          specifiers: {},
+        },
+      },
+      lockfileVersion: 5,
+    },
+    workspacePackages,
+  }))
+})

packages/supi/test/index.ts

@@ -1,4 +1,5 @@
 ///<reference path="../../../typings/index.d.ts" />
+import './allProjectsAreUpToDate.test'
 import './api'
 import './breakingChanges'
 import './cache'