feat: breaking API changes

packages/fetcher-base/src/index.ts

@@ -2,7 +2,6 @@ import { Resolution } from '@pnpm/resolver-base'
 
 export interface FetchOptions {
   cachedTarballLocation: string,
-  pkgId: string,
   prefix: string,
   onStart?: (totalSize: number | null, attempt: number) => void,
   onProgress?: (downloaded: number) => void,

packages/headless/src/runDependenciesScripts.ts

@@ -58,7 +58,7 @@ export default async (
           try {
             await opts.storeController.upload(depNode.peripheralLocation, {
               engine: ENGINE_NAME,
-              pkgId: depNode.pkgId,
+              packageId: depNode.pkgId,
             })
           } catch (err) {
             if (err && err.statusCode === 403) {

packages/package-requester/src/packageRequester.ts

@@ -5,7 +5,7 @@ import {
   FetchOptions,
   FetchResult,
 } from '@pnpm/fetcher-base'
-import pnpmLogger, { storeLogger } from '@pnpm/logger'
+import { storeLogger } from '@pnpm/logger'
 import pkgIdToFilename from '@pnpm/pkgid-to-filename'
 import { fromDir as readPkgFromDir } from '@pnpm/read-package-json'
 import {
@@ -17,7 +17,6 @@ import {
 } from '@pnpm/resolver-base'
 import {
   FetchPackageToStoreFunction,
-  LoggedPkg,
   PackageFilesResponse,
   PackageResponse,
   RequestPackageFunction,
@@ -37,7 +36,6 @@ import rimraf = require('rimraf-then')
 import symlinkDir = require('symlink-dir')
 import writeJsonFile from 'write-json-file'
 
-const dependencyResolvedLogger = pnpmLogger('_dependency_resolved')
 const TARBALL_INTEGRITY_FILENAME = 'tarball-integrity'
 
 export default function (
@@ -96,12 +94,11 @@ async function resolveAndFetch (
   options: {
     defaultTag?: string,
     downloadPriority: number,
-    loggedPkg: LoggedPkg,
-    currentPkgId?: string,
+    currentPackageId?: string,
+    currentResolution?: Resolution,
     prefix: string,
     registry: string,
     lockfileDirectory: string,
-    lockfileResolution?: Resolution,
     update?: boolean,
     verifyStoreIntegrity: boolean,
     preferredVersions: {
@@ -119,8 +116,8 @@ async function resolveAndFetch (
     let latest: string | undefined
     let pkg: PackageJson | undefined
     let normalizedPref: string | undefined
-    let resolution = options.lockfileResolution as Resolution
-    let pkgId = options.currentPkgId
+    let resolution = options.currentResolution as Resolution
+    let pkgId = options.currentPackageId
     const skipResolution = resolution && !options.update
     let forceFetch = false
     let updated = false
@@ -148,9 +145,9 @@ async function resolveAndFetch (
       // If the integrity of a local tarball dependency has changed,
       // the local tarball should be unpacked, so a fetch to the store should be forced
       forceFetch = Boolean(
-        options.lockfileResolution &&
+        options.currentResolution &&
         pkgId && pkgId.startsWith('file:') &&
-        options.lockfileResolution['integrity'] !== resolveResult.resolution['integrity'], // tslint:disable-line:no-string-literal
+        options.currentResolution['integrity'] !== resolveResult.resolution['integrity'], // tslint:disable-line:no-string-literal
       )
 
       if (!skipResolution || forceFetch) {
@@ -167,11 +164,6 @@ async function resolveAndFetch (
 
     const id = pkgId as string
 
-    dependencyResolvedLogger.debug({
-      resolution: id,
-      wanted: options.loggedPkg,
-    })
-
     if (resolution.type === 'directory') {
       if (!pkg) {
         throw new Error(`Couldn't read package.json of local dependency ${wantedDependency.alias ? wantedDependency.alias + '@' : ''}${wantedDependency.pref}`)
@@ -243,7 +235,12 @@ async function resolveAndFetch (
 
 function fetchToStore (
   ctx: {
-    fetch: FetchFunction,
+    fetch: (
+      packageId: string,
+      resolution: Resolution,
+      target: string,
+      opts: FetchOptions
+    ) => Promise<FetchResult>,
     fetchingLocker: Map<string, {
       finishing: Promise<void>,
       fetchingFiles: Promise<PackageFilesResponse>,
@@ -410,26 +407,30 @@ function fetchToStore (
           // As much tarballs should be downloaded simultaneously as possible.
           const priority = (++ctx.requestsQueue['counter'] % ctx.requestsQueue['concurrency'] === 0 ? -1 : 1) * 1000 // tslint:disable-line
 
-          const fetchedPackage = await ctx.requestsQueue.add(() => ctx.fetch(opts.resolution, target, {
-            cachedTarballLocation: path.join(ctx.storePath, opts.pkgId, 'packed.tgz'),
-            onProgress: (downloaded) => {
-              fetchingProgressLogger.debug({
-                downloaded,
-                packageId: opts.pkgId,
-                status: 'in_progress',
-              })
+          const fetchedPackage = await ctx.requestsQueue.add(() => ctx.fetch(
+            opts.pkgId,
+            opts.resolution,
+            target,
+            {
+              cachedTarballLocation: path.join(ctx.storePath, opts.pkgId, 'packed.tgz'),
+              onProgress: (downloaded) => {
+                fetchingProgressLogger.debug({
+                  downloaded,
+                  packageId: opts.pkgId,
+                  status: 'in_progress',
+                })
+              },
+              onStart: (size, attempt) => {
+                fetchingProgressLogger.debug({
+                  attempt,
+                  packageId: opts.pkgId,
+                  size,
+                  status: 'started',
+                })
+              },
+              prefix: opts.prefix,
             },
-            onStart: (size, attempt) => {
-              fetchingProgressLogger.debug({
-                attempt,
-                packageId: opts.pkgId,
-                size,
-                status: 'started',
-              })
-            },
-            pkgId: opts.pkgId,
-            prefix: opts.prefix,
-          }), { priority })
+          ), { priority })
 
           filesIndex = fetchedPackage.filesIndex
           tempLocation = fetchedPackage.tempLocation
@@ -536,6 +537,7 @@ function differed<T> (): PromiseContainer<T> {
 
 async function fetcher (
   fetcherByHostingType: {[hostingType: string]: FetchFunction},
+  packageId: string,
   resolution: Resolution,
   target: string,
   opts: FetchOptions,
@@ -547,7 +549,7 @@ async function fetcher (
   try {
     return await fetch(resolution, target, opts)
   } catch (err) {
-    storeLogger.warn(`Fetching ${opts.pkgId} failed!`)
+    storeLogger.warn(`Fetching ${packageId} failed!`)
     throw err
   }
 }

packages/package-requester/test/index.ts

@@ -50,9 +50,6 @@ test('request package', async t => {
   const pkgResponse = await requestPackage({ alias: 'is-positive', pref: '1.0.0' }, {
     downloadPriority: 0,
     lockfileDirectory: prefix,
-    loggedPkg: {
-      rawSpec: 'is-positive@1.0.0',
-    },
     preferredVersions: {},
     prefix,
     registry,
@@ -104,9 +101,6 @@ test('request package but skip fetching', async t => {
   const pkgResponse = await requestPackage({ alias: 'is-positive', pref: '1.0.0' }, {
     downloadPriority: 0,
     lockfileDirectory: prefix,
-    loggedPkg: {
-      rawSpec: 'is-positive@1.0.0',
-    },
     preferredVersions: {},
     prefix,
     registry,
@@ -149,17 +143,14 @@ test('request package but skip fetching, when resolution is already available',
 
   const prefix = tempy.directory()
   const pkgResponse = await requestPackage({ alias: 'is-positive', pref: '1.0.0' }, {
-    currentPkgId: 'registry.npmjs.org/is-positive/1.0.0',
-    downloadPriority: 0,
-    lockfileDirectory: prefix,
-    lockfileResolution: {
+    currentPackageId: 'registry.npmjs.org/is-positive/1.0.0',
+    currentResolution: {
       integrity: 'sha1-iACYVrZKLx632LsBeUGEJK4EUss=',
       registry: 'https://registry.npmjs.org/',
       tarball: 'https://registry.npmjs.org/is-positive/-/is-positive-1.0.0.tgz',
     },
-    loggedPkg: {
-      rawSpec: 'is-positive@1.0.0',
-    },
+    downloadPriority: 0,
+    lockfileDirectory: prefix,
     preferredVersions: {},
     prefix,
     registry,
@@ -207,12 +198,9 @@ test('refetch local tarball if its integrity has changed', async t => {
   const storePath = path.join(__dirname, '..', '.store')
   const pkgId = `file:${normalize(tarballRelativePath)}`
   const requestPackageOpts = {
-    currentPkgId: pkgId,
+    currentPackageId: pkgId,
     downloadPriority: 0,
     lockfileDirectory: prefix,
-    loggedPkg: {
-      rawSpec: tarball,
-    },
     preferredVersions: {},
     prefix,
     registry,
@@ -230,7 +218,7 @@ test('refetch local tarball if its integrity has changed', async t => {
 
     const response = await requestPackage(wantedPackage, {
       ...requestPackageOpts,
-      lockfileResolution: {
+      currentResolution: {
         integrity: 'sha512-lqODmYcc/FKOGROEUByd5Sbugqhzgkv+Hij9PXH0sZVQsU2npTQ0x3L81GCtHilFKme8lhBtD31Vxg/AKYrAvg==',
         tarball,
       },
@@ -257,7 +245,7 @@ test('refetch local tarball if its integrity has changed', async t => {
 
     const response = await requestPackage(wantedPackage, {
       ...requestPackageOpts,
-      lockfileResolution: {
+      currentResolution: {
         integrity: 'sha512-lqODmYcc/FKOGROEUByd5Sbugqhzgkv+Hij9PXH0sZVQsU2npTQ0x3L81GCtHilFKme8lhBtD31Vxg/AKYrAvg==',
         tarball,
       },
@@ -281,7 +269,7 @@ test('refetch local tarball if its integrity has changed', async t => {
 
     const response = await requestPackage(wantedPackage, {
       ...requestPackageOpts,
-      lockfileResolution: {
+      currentResolution: {
         integrity: 'sha512-v3uhYkN+Eh3Nus4EZmegjQhrfpdPIH+2FjrkeBc6ueqZJWWRaLnSYIkD0An6m16D3v+6HCE18ox6t95eGxj5Pw==',
         tarball,
       },
@@ -310,9 +298,6 @@ test('refetch local tarball if its integrity has changed. The requester does not
   const requestPackageOpts = {
     downloadPriority: 0,
     lockfileDirectory: prefix,
-    loggedPkg: {
-      rawSpec: tarball,
-    },
     preferredVersions: {},
     prefix,
     registry,
@@ -582,9 +567,6 @@ test('always return a package manifest in the response', async t => {
     const pkgResponse = await requestPackage({ alias: 'is-positive', pref: '1.0.0' }, {
       downloadPriority: 0,
       lockfileDirectory: prefix,
-      loggedPkg: {
-        rawSpec: 'is-positive@1.0.0',
-      },
       preferredVersions: {},
       prefix,
       registry,
@@ -597,17 +579,14 @@ test('always return a package manifest in the response', async t => {
 
   {
     const pkgResponse = await requestPackage({ alias: 'is-positive', pref: '1.0.0' }, {
-      currentPkgId: 'registry.npmjs.org/is-positive/1.0.0',
-      downloadPriority: 0,
-      lockfileDirectory: prefix,
-      lockfileResolution: {
+      currentPackageId: 'registry.npmjs.org/is-positive/1.0.0',
+      currentResolution: {
         integrity: 'sha1-iACYVrZKLx632LsBeUGEJK4EUss=',
         registry: 'https://registry.npmjs.org/',
         tarball: 'https://registry.npmjs.org/is-positive/-/is-positive-1.0.0.tgz',
       },
-      loggedPkg: {
-        rawSpec: 'is-positive@1.0.0',
-      },
+      downloadPriority: 0,
+      lockfileDirectory: prefix,
       preferredVersions: {},
       prefix,
       registry,

packages/package-store/src/storeController/index.ts

@@ -147,8 +147,8 @@ export default async function (
     return results
   }
 
-  async function upload (builtPkgLocation: string, opts: {pkgId: string, engine: string}) {
-    const cachePath = path.join(store, opts.pkgId, 'side_effects', opts.engine, 'package')
+  async function upload (builtPkgLocation: string, opts: {packageId: string, engine: string}) {
+    const cachePath = path.join(store, opts.packageId, 'side_effects', opts.engine, 'package')
     // TODO calculate integrity.json here
     const filenames: string[] = []
     await copyPkg(builtPkgLocation, cachePath, { filesResponse: { fromStore: true, filenames }, force: true })

packages/resolve-dependencies/src/resolveDependencies.ts

@@ -47,6 +47,8 @@ import semver = require('semver')
 import encodePkgId from './encodePkgId'
 import wantedDepIsLocallyAvailable from './wantedDepIsLocallyAvailable'
 
+const dependencyResolvedLogger = logger('_dependency_resolved')
+
 export function nodeIdToParents (
   nodeId: string,
   resolvedPackagesByPackageId: ResolvedPackagesByPackageId,
@@ -321,9 +323,9 @@ function getInfoFromLockfile (
   if (dependencyLockfile) {
     const depPath = dp.resolve(registries, relDepPath)
     return {
+      currentResolution: pkgSnapshotToResolution(relDepPath, dependencyLockfile, registries),
       dependencyLockfile,
       depPath,
-      lockfileResolution: pkgSnapshotToResolution(relDepPath, dependencyLockfile, registries),
       optionalDependencyNames: R.keys(dependencyLockfile.optionalDependencies),
       pkgId: packageIdFromSnapshot(relDepPath, dependencyLockfile, registries),
       relDepPath,
@@ -352,7 +354,7 @@ async function resolveDependency (
     parentNodeId: string,
     currentDepth: number,
     dependencyLockfile?: PackageSnapshot,
-    lockfileResolution?: Resolution,
+    currentResolution?: Resolution,
     resolvedDependencies?: ResolvedDependencies,
     optionalDependencyNames?: string[],
     parentIsInstallable?: boolean,
@@ -370,7 +372,7 @@ async function resolveDependency (
     options.update ||
     options.localPackages &&
     wantedDepIsLocallyAvailable(options.localPackages, wantedDependency, { defaultTag: ctx.defaultTag, registry: ctx.registries.default }))
-  const proceed = update || options.proceed || !options.lockfileResolution || ctx.force || keypath.length <= ctx.updateDepth
+  const proceed = update || options.proceed || !options.currentResolution || ctx.force || keypath.length <= ctx.updateDepth
     || options.dependencyLockfile && options.dependencyLockfile.peerDependencies
   const parentIsInstallable = options.parentIsInstallable === undefined || options.parentIsInstallable
 
@@ -388,23 +390,15 @@ async function resolveDependency (
   const scope = wantedDependency.alias && getScope(wantedDependency.alias)
   const registry = normalizeRegistry(scope && ctx.registries[scope] || ctx.registries.default)
 
-  const dependentId = keypath[keypath.length - 1]
-  const loggedPkg = {
-    dependentId,
-    name: wantedDependency.alias,
-    rawSpec: wantedDependency.raw,
-  }
-
   let pkgResponse!: PackageResponse
   try {
     pkgResponse = await ctx.storeController.requestPackage(wantedDependency, {
-      currentPkgId: options.pkgId,
+      currentPackageId: options.pkgId,
+      currentResolution: options.currentResolution,
       defaultTag: ctx.defaultTag,
       downloadPriority: -options.currentDepth,
       localPackages: options.localPackages,
       lockfileDirectory: ctx.lockfileDirectory,
-      lockfileResolution: options.lockfileResolution,
-      loggedPkg,
       preferredVersions: ctx.preferredVersions,
       prefix: ctx.prefix,
       registry,
@@ -433,6 +427,16 @@ async function resolveDependency (
     throw err
   }
 
+  const dependentId = keypath[keypath.length - 1]
+  dependencyResolvedLogger.debug({
+    resolution: pkgResponse.body.id,
+    wanted: {
+      dependentId,
+      name: wantedDependency.alias,
+      rawSpec: wantedDependency.raw,
+    },
+  })
+
   pkgResponse.body.id = encodePkgId(pkgResponse.body.id)
 
   if (!options.parentDependsOnPeer && !pkgResponse.body.updated && options.update && options.currentDepth >= ctx.updateDepth && options.relDepPath &&

packages/server/example/client.js

@@ -13,7 +13,6 @@ main()
     {alias: 'is-positive', pref: '1.0.0'},
     {
       downloadPriority: 0,
-      loggedPkg: {rawSpec: 'sfdf'},
       offline: false,
       prefix,
       registry,

packages/server/src/connectStoreController.ts

@@ -71,7 +71,7 @@ export default function (
           prefix,
         })
       },
-      upload: async (builtPkgLocation: string, opts: {pkgId: string, engine: string}) => {
+      upload: async (builtPkgLocation: string, opts: {packageId: string, engine: string}) => {
         await limitedFetch(`${remotePrefix}/upload`, {
           builtPkgLocation,
           opts,

packages/server/test/index.ts

@@ -52,7 +52,6 @@ test('server', async t => {
     {
       downloadPriority: 0,
       lockfileDirectory: prefix,
-      loggedPkg: { rawSpec: 'sfdf' },
       preferredVersions: {},
       prefix,
       registry,
@@ -149,7 +148,6 @@ test('server errors should arrive to the client', async t => {
       {
         downloadPriority: 0,
         lockfileDirectory: prefix,
-        loggedPkg: { rawSpec: 'sfdf' },
         preferredVersions: {},
         prefix,
         registry,
@@ -188,7 +186,7 @@ test('server upload', async t => {
 
   await storeCtrl.upload(path.join(__dirname, 'side-effect-fake-dir'), {
     engine: fakeEngine,
-    pkgId: fakePkgId,
+    packageId: fakePkgId,
   })
 
   const cachePath = path.join('.store', fakePkgId, 'side_effects', fakeEngine, 'package')
@@ -221,7 +219,7 @@ test('disable server upload', async t => {
   try {
     await storeCtrl.upload(path.join(__dirname, 'side-effect-fake-dir'), {
       engine: fakeEngine,
-      pkgId: fakePkgId,
+      packageId: fakePkgId,
     })
   } catch (e) {
     thrown = true
@@ -320,7 +318,6 @@ test('find package usages', async t => {
     {
       downloadPriority: 0,
       lockfileDirectory: prefix,
-      loggedPkg: { rawSpec: 'sfdf' },
       preferredVersions: {},
       prefix,
       registry,

packages/store-controller-types/src/index.ts

@@ -27,7 +27,7 @@ export interface StoreController {
   updateConnections (prefix: string, opts: {addDependencies: string[], removeDependencies: string[], prune: boolean}): Promise<void>,
   prune (): Promise<void>,
   saveState (): Promise<void>,
-  upload (builtPkgLocation: string, opts: {pkgId: string, engine: string}): Promise<void>,
+  upload (builtPkgLocation: string, opts: {packageId: string, engine: string}): Promise<void>,
   findPackageUsages (searchQueries: string[]): Promise<PackageUsagesBySearchQueries>,
 }
 
@@ -79,31 +79,24 @@ export type RequestPackageFunction = (
 ) => Promise<PackageResponse>
 
 export interface RequestPackageOptions {
+  currentPackageId?: string,
+  currentResolution?: Resolution,
   defaultTag?: string,
-  skipFetch?: boolean,
   downloadPriority: number,
-  loggedPkg: LoggedPkg,
-  currentPkgId?: string,
-  prefix: string,
-  registry: string,
   lockfileDirectory: string,
-  lockfileResolution?: Resolution,
-  update?: boolean,
-  verifyStoreIntegrity: boolean, // TODO: this should be a context field
+  localPackages?: LocalPackages,
+  prefix: string,
   preferredVersions: {
     [packageName: string]: {
       selector: string,
       type: 'version' | 'range' | 'tag',
     },
   },
-  localPackages?: LocalPackages,
+  registry: string,
   sideEffectsCache?: boolean,
-}
-
-export interface LoggedPkg {
-  rawSpec: string,
-  name?: string,
-  dependentId?: string,
+  skipFetch?: boolean,
+  update?: boolean,
+  verifyStoreIntegrity: boolean, // TODO: this should be a context field
 }
 
 export type PackageResponse = {

packages/supi/src/install/index.ts

@@ -882,7 +882,7 @@ async function installInContext (
               try {
                 await opts.storeController.upload(pkg.peripheralLocation, {
                   engine: ENGINE_NAME,
-                  pkgId: pkg.id,
+                  packageId: pkg.id,
                 })
               } catch (err) {
                 if (err && err.statusCode === 403) {

packages/supi/src/storeAdd.ts

@@ -40,9 +40,6 @@ export default async function (
       const pkgResponse = await opts.storeController.requestPackage(dep, {
         downloadPriority: 1,
         lockfileDirectory: prefix,
-        loggedPkg: {
-          rawSpec: dep.raw,
-        },
         preferredVersions: {},
         prefix,
         registry: normalizeRegistryUrl(opts.registry || 'https://registry.npmjs.org/'),

packages/tarball-fetcher/test/download.ts

@@ -48,7 +48,6 @@ test('fail when tarball size does not match content-length', async t => {
   try {
     await fetch.tarball(resolution, unpackTo, {
       cachedTarballLocation,
-      pkgId: 'registry.npmjs.org/foo/1.0.0',
       prefix: process.cwd(),
     })
     t.fail('should have failed')
@@ -88,7 +87,6 @@ test('retry when tarball size does not match content-length', async t => {
 
   const result = await fetch.tarball(resolution, unpackTo, {
     cachedTarballLocation,
-    pkgId: 'registry.npmjs.org/foo/1.0.0',
     prefix: process.cwd(),
   })
 
@@ -139,7 +137,6 @@ test('redownload incomplete cached tarballs', async t => {
   try {
     await fetch.tarball(resolution, unpackTo, {
       cachedTarballLocation,
-      pkgId: 'registry.npmjs.org/foo/1.0.0',
       prefix: process.cwd(),
     })
   } catch (err) {
@@ -173,7 +170,6 @@ test('fail when integrity check fails two times in a row', async t => {
   try {
     await fetch.tarball(resolution, unpackTo, {
       cachedTarballLocation,
-      pkgId: 'registry.npmjs.org/foo/1.0.0',
       prefix: process.cwd(),
     })
     t.fail('should have failed')
@@ -213,7 +209,6 @@ test('retry when integrity check fails', async t => {
   const params: Array<[number | null, number]> = []
   await fetch.tarball(resolution, unpackTo, {
     cachedTarballLocation,
-    pkgId: 'registry.npmjs.org/foo/1.0.0',
     prefix: process.cwd(),
     onStart (size, attempts) {
       params.push([size, attempts])
@@ -244,7 +239,6 @@ test('fail when integrity check of local file fails', async (t) => {
   try {
     await fetch.tarball(resolution, unpackTo, {
       cachedTarballLocation,
-      pkgId: `file:${tarball}`,
       prefix: process.cwd(),
     })
   } catch (_err) {
@@ -276,7 +270,6 @@ test("don't fail when integrity check of local file succeeds", async (t) => {
 
   const { filesIndex, tempLocation } = await fetch.tarball(resolution, unpackTo, {
     cachedTarballLocation,
-    pkgId: `file:${tarball}`,
     prefix: process.cwd(),
   })
 
@@ -307,7 +300,6 @@ test('retry on server error', async t => {
 
   const index = await fetch.tarball(resolution, unpackTo, {
     cachedTarballLocation,
-    pkgId: 'registry.npmjs.org/foo/1.0.0',
     prefix: process.cwd(),
   })
 
@@ -337,7 +329,6 @@ test('throw error when accessing private package w/o authorization', async t =>
   try {
     await fetch.tarball(resolution, unpackTo, {
       cachedTarballLocation,
-      pkgId: 'registry.npmjs.org/foo/1.0.0',
       prefix: process.cwd(),
     })
   } catch (_err) {
@@ -394,7 +385,6 @@ test('accessing private packages', async t => {
 
   const index = await fetch.tarball(resolution, unpackTo, {
     cachedTarballLocation,
-    pkgId: 'registry.npmjs.org/foo/1.0.0',
     prefix: process.cwd(),
   })