Skip to content

Commit

Permalink
Fix npm ci with file: dependencies
Browse files Browse the repository at this point in the history
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.

Fix: https://npm.community/t/6-8-0-npm-ci-fails-with-local-dependency/5385
Fix: https://npm.community/t/npm-ci-fail-to-local-packages/6076
PR-URL: #216
Credit: @jfirebaugh
Close: #216
Reviewed-by: @isaacs

EDIT: Updated test to not rely on network and follow latest and greatest
test patterns.
  • Loading branch information
jfirebaugh authored and isaacs committed Jul 21, 2019
1 parent 3dcf86f commit 2fb0509
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 3 deletions.
3 changes: 1 addition & 2 deletions lib/shrinkwrap.js
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
82 changes: 82 additions & 0 deletions test/tap/ci-with-local-dependency.js
@@ -0,0 +1,82 @@
const fs = require('graceful-fs')
const path = require('path')

const mkdirp = require('mkdirp')
const t = require('tap')

const common = require('../common-tap.js')

const pkg = common.pkg + '/package'

const EXEC_OPTS = {
cwd: pkg,
stdio: [0, 1, 2],
env: common.newEnv().extend({
npm_config_registry: common.registry
})
}

const localDependencyJson = {
name: 'local-dependency',
version: '0.0.0',
dependencies: {
'test-package': '0.0.0'
}
}

const dependentJson = {
name: 'dependent',
version: '0.0.0',
dependencies: {
'local-dependency': '../local-dependency'
}
}

const target = path.resolve(pkg, '../local-dependency')
const mr = require('npm-registry-mock')
let server
t.teardown(() => {
if (server) {
server.close()
}
})

t.test('setup', function (t) {
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)
)
mr({ port: common.port }, (er, s) => {
if (er) {
throw er
}
server = s
t.end()
})
})

t.test('\'npm install\' should install local pkg from sub path', function (t) {
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/test-package')).isDirectory(), 'transitive dependency installed')
t.end()
})
})

t.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/test-package')).isDirectory(), 'transitive dependency installed')
t.end()
})
})
4 changes: 3 additions & 1 deletion test/tap/spec-local-specifiers.js
Expand Up @@ -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()
})
})
Expand Down

0 comments on commit 2fb0509

Please sign in to comment.