From 262c99738524ccb6211294f94e40fc21cc4081bf Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Thu, 18 Jul 2019 15:35:20 -0700 Subject: [PATCH] Fix `npm ci` with `file:` dependencies Partially reverts #40/#86, keeping the "Don't record linked deps as bundled" part but reverting the "Don't iterate into linked deps" part. It seems that we need to record dependencies of linked deps in order for `npm ci` to work. Fixes https://npm.community/t/6-8-0-npm-ci-fails-with-local-dependency/5385 Fixes https://npm.community/t/npm-ci-fail-to-local-packages/6076 --- lib/shrinkwrap.js | 3 +- test/tap/ci-with-local-dependency.js | 83 ++++++++++++++++++++++++++++ test/tap/spec-local-specifiers.js | 4 +- 3 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 test/tap/ci-with-local-dependency.js diff --git a/lib/shrinkwrap.js b/lib/shrinkwrap.js index 75d58bf8e4f0b..bd8c0abbaa205 100644 --- a/lib/shrinkwrap.js +++ b/lib/shrinkwrap.js @@ -111,7 +111,6 @@ function shrinkwrapDeps (deps, top, tree, seen) { var pkginfo = deps[moduleName(child)] = {} var requested = getRequested(child) || child.package._requested || {} var linked = child.isLink || child.isInLink - var linkedFromHere = linked && path.relative(top.realpath, child.realpath)[0] !== '.' pkginfo.version = childVersion(top, child, requested) if (requested.type === 'git' && child.package._from) { pkginfo.from = child.package._from @@ -142,7 +141,7 @@ function shrinkwrapDeps (deps, top, tree, seen) { }) } // iterate into children on non-links and links contained within the top level package - if (child.children.length && (!child.isLink || linkedFromHere)) { + if (child.children.length) { pkginfo.dependencies = {} shrinkwrapDeps(pkginfo.dependencies, top, child, seen) } diff --git a/test/tap/ci-with-local-dependency.js b/test/tap/ci-with-local-dependency.js new file mode 100644 index 0000000000000..10efedee40063 --- /dev/null +++ b/test/tap/ci-with-local-dependency.js @@ -0,0 +1,83 @@ +var fs = require('graceful-fs') +var path = require('path') + +var mkdirp = require('mkdirp') +var osenv = require('osenv') +var rimraf = require('rimraf') +var test = require('tap').test + +var common = require('../common-tap.js') + +var pkg = common.pkg + +var EXEC_OPTS = { cwd: pkg, stdio: [0, 1, 2] } + +var localDependencyJson = { + name: 'local-dependency', + version: '0.0.0', + dependencies: { + underscore: '1.5.1' + } +} + +var dependentJson = { + name: 'dependent', + version: '0.0.0', + dependencies: { + 'local-dependency': '../local-dependency' + } +} + +var target = path.resolve(pkg, '../local-dependency') + +test('setup', function (t) { + cleanup() + t.end() +}) + +test('\'npm install\' should install local pkg from sub path', function (t) { + setup() + common.npm(['install', '--loglevel=silent'], EXEC_OPTS, function (err, code) { + if (err) throw err + t.equal(code, 0, 'npm install exited with code') + t.ok(fs.statSync(path.resolve(pkg, 'node_modules/local-dependency/package.json')).isFile(), 'local dependency package.json exists') + t.ok(fs.statSync(path.resolve(pkg, 'node_modules/local-dependency/node_modules/underscore')).isDirectory(), 'transitive dependency installed') + t.end() + }) +}) + +test('\'npm ci\' should work', function (t) { + common.npm(['ci', '--loglevel=silent'], EXEC_OPTS, function (err, code) { + if (err) throw err + t.equal(code, 0, 'npm install exited with code') + t.ok(fs.statSync(path.resolve(pkg, 'node_modules/local-dependency/package.json')).isFile(), 'local dependency package.json exists') + t.ok(fs.statSync(path.resolve(pkg, 'node_modules/local-dependency/node_modules/underscore')).isDirectory(), 'transitive dependency installed') + t.end() + }) +}) + +test('cleanup', function (t) { + cleanup() + t.end() +}) + +function cleanup () { + process.chdir(osenv.tmpdir()) + rimraf.sync(pkg) + rimraf.sync(target) +} + +function setup () { + cleanup() + mkdirp.sync(target) + fs.writeFileSync( + path.join(target, 'package.json'), + JSON.stringify(localDependencyJson, null, 2) + ) + mkdirp.sync(pkg) + fs.writeFileSync( + path.join(pkg, 'package.json'), + JSON.stringify(dependentJson, null, 2) + ) + process.chdir(pkg) +} diff --git a/test/tap/spec-local-specifiers.js b/test/tap/spec-local-specifiers.js index 7b6cacf12cb69..16fac002566fa 100644 --- a/test/tap/spec-local-specifiers.js +++ b/test/tap/spec-local-specifiers.js @@ -585,7 +585,9 @@ test('save behavior', function (t) { var deps = pjson.dependencies || {} t.is(deps['sb-transitive'], 'file:../sb-transitive', 'package.json') var sdep = shrinkwrap.dependencies['sb-transitive'] || {} - t.like(sdep, {bundled: null, dependencies: null, version: 'file:../sb-transitive'}, 'npm-shrinkwrap.json direct dep') + var tdep = sdep.dependencies.sbta + t.like(tdep, {bundled: null, version: 'file:../sb-transitive/sbta'}, 'npm-shrinkwrap.json transitive dep') + t.like(sdep, {bundled: null, version: 'file:../sb-transitive'}, 'npm-shrinkwrap.json direct dep') t.done() }) })