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 not in node_modules if not in private shr
Browse files Browse the repository at this point in the history
  • Loading branch information
zkochan committed Jul 3, 2017
1 parent 5444e79 commit f820700
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 9 deletions.
16 changes: 8 additions & 8 deletions src/api/install.ts
Expand Up @@ -79,6 +79,7 @@ export type InstallContext = {
installable: boolean,
}[],
shrinkwrap: Shrinkwrap,
privateShrinkwrap: Shrinkwrap,
fetchingLocker: {
[pkgId: string]: {
fetchingFiles: Promise<PackageContentInfo>,
Expand Down Expand Up @@ -113,7 +114,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, ctx.storeIndex)
const installCtx = await createInstallCmd(ctx, ctx.skipped)

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

Expand Down Expand Up @@ -242,7 +243,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, ctx.storeIndex)
const installCtx = await createInstallCmd(ctx, ctx.skipped)

if (ctx.shrinkwrap.dependencies) {
for (const spec of packagesToInstall) {
Expand Down Expand Up @@ -554,21 +555,20 @@ function getSaveSpec(spec: PackageSpec, version: string, saveExact: boolean) {
}

async function createInstallCmd (
opts: StrictPnpmOptions,
shrinkwrap: Shrinkwrap,
skipped: Set<string>,
storeIndex: Store
ctx: PnpmContext,
skipped: Set<string>
): Promise<InstallContext> {
return {
installs: {},
localPackages: [],
childrenIdsByParentId: {},
nodesToBuild: [],
shrinkwrap,
shrinkwrap: ctx.shrinkwrap,
privateShrinkwrap: ctx.privateShrinkwrap,
fetchingLocker: {},
skipped,
tree: {},
storeIndex,
storeIndex: ctx.storeIndex,
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/install/installMultiple.ts
Expand Up @@ -151,7 +151,12 @@ async function install (
const proceed = options.force || keypath.length <= options.depth
const parentIsInstallable = options.parentIsInstallable === undefined || options.parentIsInstallable

if (!proceed && options.pkgId && await exists(path.join(options.nodeModules, `.${options.pkgId}`))) {
if (!proceed && options.pkgId &&
// if package is not in `node_modules/.shrinkwrap.yaml`
// we can safely assume that it doesn't exist in `node_modules`
ctx.privateShrinkwrap.packages && ctx.privateShrinkwrap.packages[options.pkgId] &&
await exists(path.join(options.nodeModules, `.${options.pkgId}`))) {

return null
}

Expand Down
13 changes: 13 additions & 0 deletions test/shrinkwrap.ts
Expand Up @@ -406,3 +406,16 @@ test('scoped module from different registry', async function (t: tape.Test) {
shrinkwrapVersion: 3
})
})

test('repeat install with no inner shrinkwrap should not rewrite packages in node_modules', async (t: tape.Test) => {
const project = prepare(t)

await installPkgs(['is-negative@1.0.0'], testDefaults())

await rimraf('node_modules/.shrinkwrap.yaml')

await install(testDefaults())

const m = project.requireModule('is-negative')
t.ok(m)
})

0 comments on commit f820700

Please sign in to comment.