Skip to content
This repository has been archived by the owner on May 14, 2018. It is now read-only.

Commit

Permalink
perf: assume package is not in store if it is not in store.json
Browse files Browse the repository at this point in the history
  • Loading branch information
zkochan committed Jul 3, 2017
1 parent b8541a4 commit 2881eb4
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 7 deletions.
14 changes: 11 additions & 3 deletions src/api/install.ts
Expand Up @@ -37,6 +37,7 @@ import {PackageSpec, DirectoryResolution, Resolution} from '../resolve'
import {DependencyTreeNode} from '../link/resolvePeers'
import depsToSpecs, {similarDepsToSpecs} from '../depsToSpecs'
import streamParser from '../logging/streamParser'
import {Store} from '../fs/storeController'

export type InstalledPackages = {
[name: string]: InstalledPackage
Expand Down Expand Up @@ -88,6 +89,7 @@ export type InstallContext = {
// the IDs of packages that are not installable
skipped: Set<string>,
tree: {[nodeId: string]: TreeNode},
storeIndex: Store,
}

export async function install (maybeOpts?: PnpmOptions) {
Expand All @@ -111,7 +113,7 @@ export async function install (maybeOpts?: PnpmOptions) {
async function _install() {
const installType = 'general'
const ctx = await getContext(opts, installType)
const installCtx = await createInstallCmd(opts, ctx.shrinkwrap, ctx.skipped)
const installCtx = await createInstallCmd(opts, ctx.shrinkwrap, ctx.skipped, ctx.storeIndex)

if (!ctx.pkg) throw new Error('No package.json found')

Expand Down Expand Up @@ -240,7 +242,7 @@ export async function installPkgs (fuzzyDeps: string[] | Dependencies, maybeOpts
if (!Object.keys(packagesToInstall).length) {
throw new Error('At least one package has to be installed')
}
const installCtx = await createInstallCmd(opts, ctx.shrinkwrap, ctx.skipped)
const installCtx = await createInstallCmd(opts, ctx.shrinkwrap, ctx.skipped, ctx.storeIndex)

if (ctx.shrinkwrap.dependencies) {
for (const spec of packagesToInstall) {
Expand Down Expand Up @@ -551,7 +553,12 @@ function getSaveSpec(spec: PackageSpec, version: string, saveExact: boolean) {
}
}

async function createInstallCmd (opts: StrictPnpmOptions, shrinkwrap: Shrinkwrap, skipped: Set<string>): Promise<InstallContext> {
async function createInstallCmd (
opts: StrictPnpmOptions,
shrinkwrap: Shrinkwrap,
skipped: Set<string>,
storeIndex: Store
): Promise<InstallContext> {
return {
installs: {},
localPackages: [],
Expand All @@ -561,6 +568,7 @@ async function createInstallCmd (opts: StrictPnpmOptions, shrinkwrap: Shrinkwrap
fetchingLocker: {},
skipped,
tree: {},
storeIndex,
}
}

Expand Down
15 changes: 13 additions & 2 deletions src/install/fetch.ts
Expand Up @@ -14,6 +14,7 @@ import writeJsonFile = require('write-json-file')
import pkgIdToFilename from '../fs/pkgIdToFilename'
import {fromDir as readPkgFromDir} from '../fs/readPkg'
import {fromDir as safeReadPkgFromDir} from '../fs/safeReadPkg'
import {Store} from '../fs/storeController'
import exists = require('path-exists')
import memoize, {MemoizedFunc} from '../memoize'
import {Package} from '../types'
Expand Down Expand Up @@ -60,6 +61,7 @@ export default async function fetch (
},
loggedPkg: LoggedPkg,
offline: boolean,
storeIndex: Store,
}
): Promise<FetchedPackage> {
try {
Expand Down Expand Up @@ -101,17 +103,20 @@ export default async function fetch (
}
}

const target = path.join(options.storePath, pkgIdToFilename(id))
const targetRelative = pkgIdToFilename(id)
const target = path.join(options.storePath, targetRelative)

if (!options.fetchingLocker[id]) {
options.fetchingLocker[id] = fetchToStore({
target,
targetRelative,
resolution: <Resolution>resolution,
pkgId: id,
got: options.got,
storePath: options.storePath,
offline: options.offline,
pkg,
storeIndex: options.storeIndex,
})
}

Expand All @@ -132,12 +137,14 @@ export default async function fetch (

function fetchToStore (opts: {
target: string,
targetRelative: string,
resolution: Resolution,
pkgId: string,
got: Got,
storePath: string,
offline: boolean,
pkg?: Package,
storeIndex: Store,
}): {
fetchingFiles: Promise<PackageContentInfo>,
fetchingPkg: Promise<Package>,
Expand All @@ -164,7 +171,11 @@ function fetchToStore (opts: {

let target = opts.target
const linkToUnpacked = path.join(target, 'package')
const targetExists = await exists(path.join(linkToUnpacked, 'package.json'))

// We can safely assume that if there is no data about the package in `store.json` then
// it is not in the store yet.
// In case there is record about the package in `store.json`, we check it in the file system just in case
const targetExists = opts.storeIndex[opts.targetRelative] && await exists(path.join(linkToUnpacked, 'package.json'))

if (targetExists) {
// if target exists and it wasn't modified, then no need to refetch it
Expand Down
1 change: 1 addition & 0 deletions src/install/installMultiple.ts
Expand Up @@ -180,6 +180,7 @@ async function install (
shrinkwrapResolution: options.shrinkwrapResolution,
pkgId: options.pkgId,
offline: options.offline,
storeIndex: ctx.storeIndex,
})

if (fetchedPkg.isLocal) {
Expand Down
4 changes: 2 additions & 2 deletions test/shrinkwrap.ts
Expand Up @@ -324,15 +324,15 @@ test('repeat install with shrinkwrap should not mutate shrinkwrap when dependenc

const shr1 = await project.loadShrinkwrap()

t.equal(shr1.dependencies['highmaps-release'], '5.0.11')
t.equal(shr1.dependencies['highmaps-release'], '5.0.11', 'dependency added correctly to shrinkwrap.yaml')

await rimraf('node_modules')

await install(testDefaults())

const shr2 = await project.loadShrinkwrap()

t.deepEqual(shr1, shr2)
t.deepEqual(shr1, shr2, "shrinkwrap file hasn't been changed")
})

test('package is not marked dev if it is also a subdep of a regular dependency', async (t: tape.Test) => {
Expand Down

0 comments on commit 2881eb4

Please sign in to comment.