From 08e096a06be02543bca465e112fd62ac9ac33e49 Mon Sep 17 00:00:00 2001 From: zkochan Date: Fri, 28 Jul 2017 00:11:39 +0300 Subject: [PATCH] fix: don't update package when unlinking --- src/api/install.ts | 13 +++----- src/api/unlink.ts | 10 ++++-- src/install/installMultiple.ts | 60 ++++++++++++++++++++++------------ test/shrinkwrap.ts | 2 +- test/unlink.ts | 23 +++++++++++++ 5 files changed, 75 insertions(+), 33 deletions(-) diff --git a/src/api/install.ts b/src/api/install.ts index dce923649c..aba9e14d83 100644 --- a/src/api/install.ts +++ b/src/api/install.ts @@ -219,6 +219,8 @@ export async function installPkgs (fuzzyDeps: string[] | Dependencies, maybeOpts streamParser.on('data', reporter) } + maybeOpts = maybeOpts || {} + if (maybeOpts.update === undefined) maybeOpts.update = true const opts = extendOptions(maybeOpts) if (opts.lock) { @@ -262,12 +264,6 @@ export async function installPkgs (fuzzyDeps: string[] | Dependencies, maybeOpts } const installCtx = await createInstallCmd(ctx, ctx.skipped) - if (ctx.shrinkwrap.dependencies) { - for (const spec of packagesToInstall) { - delete ctx.shrinkwrap.dependencies[spec.name] - } - } - if (opts.lock === false) { return run() } @@ -340,13 +336,12 @@ async function installInContext ( const oldSpecs = parts[0] const newSpecs = parts[1] - const update = opts.update || installType === 'named' const installOpts = { root: ctx.root, storePath: ctx.storePath, registry: ctx.shrinkwrap.registry, force: opts.force, - depth: update ? opts.depth : + depth: opts.update ? opts.depth : (R.equals(ctx.shrinkwrap.packages, ctx.privateShrinkwrap.packages) ? opts.repeatInstallDepth : Infinity), engineStrict: opts.engineStrict, nodeVersion: opts.nodeVersion, @@ -362,7 +357,7 @@ async function installInContext ( offline: opts.offline, rawNpmConfig: opts.rawNpmConfig, nodeModules: nodeModulesPath, - update, + update: opts.update, keypath: [], prefix: opts.prefix, parentNodeId: ':/:', diff --git a/src/api/unlink.ts b/src/api/unlink.ts index efeceb368a..c8cc664124 100644 --- a/src/api/unlink.ts +++ b/src/api/unlink.ts @@ -21,7 +21,7 @@ export async function unlinkPkgs ( if (reporter) { streamParser.on('data', reporter) } - const opts = extendOptions(maybeOpts) + const opts = _extendOptions(maybeOpts) const modulesYaml = await readModules(opts.prefix) opts.store = modulesYaml && modulesYaml.store || opts.store @@ -62,7 +62,7 @@ export async function unlink (maybeOpts: PnpmOptions) { if (reporter) { streamParser.on('data', reporter) } - const opts = extendOptions(maybeOpts) + const opts = _extendOptions(maybeOpts) const modulesYaml = await readModules(opts.prefix) opts.store = modulesYaml && modulesYaml.store || opts.store @@ -108,3 +108,9 @@ async function isExternalLink (store: string, modules: string, pkgName: string) // because packages are linked to store when independent-leaves = true return !link.isInner && !isSubdir(store, link.target) } + +function _extendOptions (maybeOpts: PnpmOptions): StrictPnpmOptions { + maybeOpts = maybeOpts || {} + if (maybeOpts.depth === undefined) maybeOpts.depth = -1 + return extendOptions(maybeOpts) +} diff --git a/src/install/installMultiple.ts b/src/install/installMultiple.ts index 43c8ffaec6..4ef8877f35 100644 --- a/src/install/installMultiple.ts +++ b/src/install/installMultiple.ts @@ -68,7 +68,7 @@ export default async function installMultiple ( nodeVersion: string, pnpmVersion: string, offline: boolean, - isInstallable?: boolean, + parentIsInstallable?: boolean, rawNpmConfig: Object, nodeModules: string, update: boolean, @@ -76,6 +76,7 @@ export default async function installMultiple ( } ): Promise { const resolvedDependencies = options.resolvedDependencies || {} + const update = options.update && options.currentDepth <= options.depth const pkgAddresses = ( await Promise.all( specs @@ -84,6 +85,9 @@ export default async function installMultiple ( return await install(spec, ctx, Object.assign({}, options, + { + update + }, getInfoFromShrinkwrap(ctx.shrinkwrap, reference, spec.name, options.registry))) }) ) @@ -100,25 +104,23 @@ function getInfoFromShrinkwrap ( registry: string, ) { if (!reference || !pkgName) { - return { - resolvedDependencies: {}, - } + return null } const dependencyPath = dp.refToRelative(reference, pkgName) if (!dependencyPath) { - return { - resolvedDependencies: {}, - } + return null } const dependencyShrinkwrap = shrinkwrap.packages && shrinkwrap.packages[dependencyPath] if (dependencyShrinkwrap) { + const absoluteDependencyPath = dp.resolve(shrinkwrap.registry, dependencyPath) return { dependencyPath, - pkgId: dependencyShrinkwrap.id || dp.resolve(shrinkwrap.registry, dependencyPath), + absoluteDependencyPath, + pkgId: dependencyShrinkwrap.id || absoluteDependencyPath, shrinkwrapResolution: dependencyShrToResolution(dependencyPath, dependencyShrinkwrap, shrinkwrap.registry), resolvedDependencies: Object.assign({}, dependencyShrinkwrap.dependencies, dependencyShrinkwrap.optionalDependencies), @@ -127,7 +129,6 @@ function getInfoFromShrinkwrap ( return { dependencyPath, pkgId: dp.resolve(shrinkwrap.registry, dependencyPath), - resolvedDependencies: {}, } } } @@ -166,6 +167,7 @@ async function install ( got: Got, keypath: string[], // TODO: remove. Currently used only for logging pkgId?: string, + absoluteDependencyPath?: string, dependencyPath?: string, parentNodeId: string, currentDepth: number, @@ -184,14 +186,16 @@ async function install ( } ): Promise { const keypath = options.keypath || [] - const proceed = options.force || keypath.length <= options.depth + const proceed = !options.resolvedDependencies || options.force || keypath.length <= options.depth const parentIsInstallable = options.parentIsInstallable === undefined || options.parentIsInstallable - if (!proceed && options.pkgId && + if (!proceed && options.absoluteDependencyPath && // if package is not in `node_modules/.shrinkwrap.yaml` // we can safely assume that it doesn't exist in `node_modules` options.dependencyPath && ctx.privateShrinkwrap.packages && ctx.privateShrinkwrap.packages[options.dependencyPath] && - await exists(path.join(options.nodeModules, `.${options.pkgId}`))) { + await exists(path.join(options.nodeModules, `.${options.absoluteDependencyPath}`)) && ( + options.currentDepth > 0 || await exists(path.join(options.nodeModules, spec.name)) + )) { return null } @@ -310,13 +314,31 @@ async function install ( const children = await installDependencies( pkg, spec, - fetchedPkg.id, ctx, - Object.assign({}, options, { + { parentIsInstallable: installable, currentDepth: options.currentDepth + 1, parentNodeId: nodeId, - }) + keypath: options.keypath.concat([ fetchedPkg.id ]), + force: options.force, + prefix: options.prefix, + storePath: options.storePath, + registry: options.registry, + metaCache: options.metaCache, + got: options.got, + resolvedDependencies: fetchedPkg.id !== options.pkgId + ? undefined + : options.resolvedDependencies, + depth: options.depth, + engineStrict: options.engineStrict, + nodeVersion: options.nodeVersion, + pnpmVersion: options.pnpmVersion, + offline: options.offline, + rawNpmConfig: options.rawNpmConfig, + nodeModules: options.nodeModules, + update: options.update, + verifyStoreInegrity: options.verifyStoreInegrity, + } ) ctx.childrenIdsByParentId[fetchedPkg.id] = children.map(child => child.pkgId) ctx.tree[nodeId] = { @@ -358,7 +380,6 @@ function normalizeRegistry (registry: string) { async function installDependencies ( pkg: Package, parentSpec: PackageSpec, - pkgId: string, ctx: InstallContext, opts: { force: boolean, @@ -376,16 +397,13 @@ async function installDependencies ( nodeVersion: string, pnpmVersion: string, offline: boolean, - isInstallable?: boolean, + parentIsInstallable: boolean, rawNpmConfig: Object, nodeModules: string, update: boolean, verifyStoreInegrity: boolean, } ): Promise { - const depsInstallOpts = Object.assign({}, opts, { - keypath: opts.keypath.concat([ pkgId ]), - }) const bundledDeps = pkg.bundleDependencies || pkg.bundledDependencies || [] const filterDeps = getNotBundledDeps.bind(null, bundledDeps) @@ -398,7 +416,7 @@ async function installDependencies ( } ) - return await installMultiple(ctx, deps, depsInstallOpts) + return await installMultiple(ctx, deps, opts) } function getNotBundledDeps (bundledDeps: string[], deps: Dependencies) { diff --git a/test/shrinkwrap.ts b/test/shrinkwrap.ts index 7af9d3d385..3e6402a7fa 100644 --- a/test/shrinkwrap.ts +++ b/test/shrinkwrap.ts @@ -113,7 +113,7 @@ test('fail when shasum from shrinkwrap does not match with the actual one', asyn } }) -test("shrinkwrap doesn't lock subdependencies that don't satisfy the new specs", async t => { +test("shrinkwrap doesn't lock subdependencies that don't satisfy the new specs", async (t: tape.Test) => { const project = prepare(t) // dependends on react-onclickoutside@5.9.0 diff --git a/test/unlink.ts b/test/unlink.ts index 014a6b9b3f..893cfbf002 100644 --- a/test/unlink.ts +++ b/test/unlink.ts @@ -4,6 +4,7 @@ import { prepare, testDefaults, pathToLocalPkg, + addDistTag, } from './utils' import writeJsonFile = require('write-json-file') import { @@ -52,6 +53,28 @@ test('unlink 1 package that exists in package.json', async (t: tape.Test) => { t.notOk((await isInnerLink('node_modules', 'is-positive')).isInner, 'is-positive left linked') }) +test("don't update package when unlinking", async (t: tape.Test) => { + const project = prepare(t) + + await addDistTag('foo', '100.0.0', 'latest') + await installPkgs(['foo'], testDefaults()) + + process.chdir('..') + + await writeJsonFile('foo/package.json', { + name: 'foo', + version: '100.0.0', + }) + + await link('foo', 'project') + await addDistTag('foo', '100.1.0', 'latest') + + process.chdir('project') + await unlinkPkgs(['foo'], testDefaults()) + + t.equal(project.requireModule('foo/package.json').version, '100.0.0', 'foo not updated after unlink') +}) + test('unlink 2 packages. One of them exists in package.json', async (t: tape.Test) => { const project = prepare(t, { dependencies: {