Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Commit

Permalink
Support acceptDependencies object in package.json
Browse files Browse the repository at this point in the history
This treats any dep ranges in the acceptDependencies field as an
acceptable alternative for any dependency range specified elsewhere in
the package manifest.

Implementation of npm/rfcs#72
  • Loading branch information
isaacs committed Feb 7, 2020
1 parent 5465693 commit 705578d
Show file tree
Hide file tree
Showing 13 changed files with 167 additions and 24 deletions.
4 changes: 3 additions & 1 deletion lib/dep-valid.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,6 @@ const depValid = (child, requested, requestor) => {
return false
}

module.exports = depValid
module.exports = (child, requested, accept, requestor) =>
depValid(child, requested, requestor) ||
(typeof accept === 'string' ? depValid(child, accept, requestor) : false)
17 changes: 14 additions & 3 deletions lib/edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const _from = Symbol('_from')
const _to = Symbol('_to')
const _type = Symbol('_type')
const _spec = Symbol('_spec')
const _accept = Symbol('_accept')
const _name = Symbol('_name')
const _error = Symbol('_error')
const _loadError = Symbol('_loadError')
Expand All @@ -21,12 +22,18 @@ const types = new Set([

class Edge {
constructor (options) {
const { type, name, spec, from } = options
const { type, name, spec, accept, from } = options

if (typeof spec !== 'string')
throw new TypeError('must provide string spec')
this[_spec] = spec

if (accept !== undefined) {
if (typeof accept !== 'string')
throw new TypeError('accept field must be a string if provided')
this[_accept] = accept || '*'
}

if (typeof name !== 'string')
throw new TypeError('must provide dependency name')
this[_name] = name
Expand All @@ -44,7 +51,7 @@ class Edge {
}

satisfiedBy (node) {
return depValid(node, this.spec, this.from)
return depValid(node, this.spec, this.accept, this.from)
}

get optional () {
Expand All @@ -67,6 +74,10 @@ class Edge {
return this[_spec]
}

get accept () {
return this[_accept]
}

get valid () {
return !this.error
}
Expand All @@ -82,7 +93,7 @@ class Edge {
this.from === this.to.parent &&
!this.from.isTop
? 'PEER LOCAL'
: !depValid(this.to, this.spec, this.from)
: !depValid(this.to, this.spec, this.accept, this.from)
? 'INVALID'
: 'OK'
}
Expand Down
4 changes: 3 additions & 1 deletion lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,13 @@ class Node {

[_loadDepType] (obj, type) {
const from = this
const ad = this.package.acceptDependencies || {}
for (const [name, spec] of Object.entries(obj || {})) {
const accept = ad[name]
// if it's already set, then we keep the existing edge
// NB: the Edge ctor adds itself to from.edgesOut
if (!this.edgesOut.get(name)) {
new Edge({ from, name, spec, type })
new Edge({ from, name, spec, accept, type })
}
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const pkgMetaKeys = [
'peerDependenciesMeta',
'optionalDependencies',
'bundleDependencies',
'acceptDependencies',
'funding',
'engines',
'os',
Expand Down
1 change: 1 addition & 0 deletions scripts/lib/print-edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const printEdge = (edge, inout) => ({
//name: edge.name,
type: edge.type,
spec: edge.spec,
...(edge.accept ? { accept: edge.accept } : {}),
...(inout === 'in' ? {
from: edge.from.location,
} : {
Expand Down
1 change: 1 addition & 0 deletions tap-snapshots/test-edge.js-TAP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Edge {
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"c" => Edge {},
"b" => Edge {},
},
"isTop": false,
Expand Down
44 changes: 25 additions & 19 deletions test/dep-valid.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,33 @@ const t = require('tap')
const depValid = require('../lib/dep-valid.js')
const npa = require('npm-package-arg')

t.ok(depValid({}, '', {}), '* is always ok')
t.ok(depValid({}, '', null, {}), '* is always ok')

t.ok(depValid({
package: {
version: '1.2.3',
},
}, '1.x', {}), 'range that is satisfied')
}, '1.x', null, {}), 'range that is satisfied')

t.ok(depValid({
package: {
version: '2.2.3',
},
}, '1.x', '2.x', {}), 'range that is acceptable')

t.ok(depValid({
isLink: true,
realpath: '/some/path'
}, npa('file:/some/path'), {}), 'links must point at intended target')
}, npa('file:/some/path'), null, {}), 'links must point at intended target')

t.notOk(depValid({
isLink: true,
realpath: '/some/other/path'
}, 'file:/some/path', {}), 'links must point at intended target')
}, 'file:/some/path', null, {}), 'links must point at intended target')

t.notOk(depValid({
realpath: '/some/path'
}, 'file:/some/path', {}), 'file:// must be a link')
}, 'file:/some/path', null, {}), 'file:// must be a link')


t.ok(depValid({
Expand All @@ -31,59 +37,59 @@ t.ok(depValid({
package: {
version: '1.2.3',
},
}, 'git://host/repo#semver:1.x', {}), 'git url with semver range')
}, 'git://host/repo#semver:1.x', null, {}), 'git url with semver range')

t.ok(depValid({
name: 'foo',
package: {
name: 'bar',
version: '1.2.3',
},
}, 'npm:[email protected]', {}), 'alias is ok')
}, 'npm:[email protected]', null, {}), 'alias is ok')

t.ok(depValid({
resolved: 'https://registry/abbrev-1.1.1.tgz',
package: {},
}, 'https://registry/abbrev-1.1.1.tgz', {}), 'remote url match')
}, 'https://registry/abbrev-1.1.1.tgz', null, {}), 'remote url match')

t.ok(depValid({
resolved: 'git+ssh://[email protected]/foo/bar',
package: {},
}, 'git+ssh://[email protected]/foo/bar.git', {}), 'matching _from saveSpec')
}, 'git+ssh://[email protected]/foo/bar.git', null, {}), 'matching _from saveSpec')

t.notOk(depValid({
resolved: 'git+ssh://[email protected]/foo/bar',
package: {},
}, 'git+ssh://[email protected]/bar/foo.git', {}), 'different repo')
}, 'git+ssh://[email protected]/bar/foo.git', null, {}), 'different repo')

t.notOk(depValid({
package: {},
}, 'git+ssh://[email protected]/bar/foo.git', {}), 'missing repo')
}, 'git+ssh://[email protected]/bar/foo.git', null, {}), 'missing repo')

t.ok(depValid({
resolved: '/path/to/tarball.tgz',
}, '/path/to/tarball.tgz', {}), 'same tarball')
}, '/path/to/tarball.tgz', null, {}), 'same tarball')

t.notOk(depValid({
resolved: '/path/to/other/tarball.tgz',
}, '/path/to/tarball.tgz', {}), 'different tarball')
}, '/path/to/tarball.tgz', null, {}), 'different tarball')

t.ok(depValid({
resolved: 'https://registry.npmjs.org/foo/foo-1.2.3.tgz',
}, 'latest', {}), 'tagged registry version needs remote tarball')
}, 'latest', null, {}), 'tagged registry version needs remote tarball')

t.notOk(depValid({
resolved: 'git+https://registry.npmjs.org/foo/foo-1.2.3.git',
}, 'latest', {}), 'tagged registry version needs remote tarball, not git')
}, 'latest', null, {}), 'tagged registry version needs remote tarball, not git')

t.notOk(depValid({}, 'latest', {}),
t.notOk(depValid({}, 'latest', null, {}),
'tagged registry version needs remote tarball resolution')

t.test('unsupported dependency type', t => {
const requestor = { errors: [] }
const child = {name: 'kid'}
const request = { type: 'not a type' }
t.notOk(depValid(child, request, requestor))
t.notOk(depValid(child, request, null, requestor))
t.match(requestor, {
errors: [{
message: 'Unsupported dependency type',
Expand All @@ -98,7 +104,7 @@ t.test('invalid tag name', t => {
const requestor = { errors: [] }
const child = { name: 'kid' }
const request = '!!@#$%!#@$!'
t.notOk(depValid(child, request, requestor))
t.notOk(depValid(child, request, null, requestor))
t.match(requestor, {
errors: [{
message: 'Invalid tag name "!!@#$%!#@$!"',
Expand All @@ -113,7 +119,7 @@ t.test('invalid request all together', t => {
const requestor = { errors: [] }
const child = { name: 'kid' }
const request = null
t.notOk(depValid(child, request, requestor))
t.notOk(depValid(child, request, null, requestor))
t.match(requestor, {
errors: [{
message: 'Invalid dependency specifier',
Expand Down
41 changes: 41 additions & 0 deletions test/edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,23 @@ const aa = {
},
}

const c = {
edgesOut: new Map(),
edgesIn: new Set(),
package: { name: 'c', version: '2.3.4' },
isTop: false,
parent: top,
resolve (n) {
return this.parent.resolve(n)
},
addEdgeOut (edge) {
this.edgesOut.set(edge.name, edge)
},
addEdgeIn (edge) {
this.edgesIn.add(edge)
},
}

t.matchSnapshot(new Edge({
from: top,
type: 'peer',
Expand Down Expand Up @@ -156,6 +173,22 @@ t.ok(new Edge({
reset(a)
reset(b)

t.ok(new Edge({
from: a,
type: 'prod',
name: 'c',
spec: '1.x',
accept: '2.x',
}).satisfiedBy(c), 'c@2 satisfies spec:1.x, accept:2.x')

t.ok(new Edge({
from: a,
type: 'prod',
name: 'c',
spec: '1.x',
accept: '',
}).satisfiedBy(c), 'c@2 satisfies spec:1.x, accept:*')

const old = new Edge({
from: a,
type: 'peer',
Expand Down Expand Up @@ -216,6 +249,14 @@ t.throws(() => new Edge({
spec: { yoinks: '1.2.3' },
}), new TypeError('must provide string spec'))

t.throws(() => new Edge({
from: top,
type: 'prod',
name: 'yoinks',
spec: '1.x',
accept: { yoinks: '2.2.3' },
}), new TypeError('accept field must be a string if provided'))

t.throws(() => new Edge({
from: top,
type: 'not a valid type',
Expand Down
11 changes: 11 additions & 0 deletions test/fixtures/accept-dependencies/dep/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "@isaacs/testing-accept-dependencies-dep",
"version": "1.0.0",
"description": "a test of acceptDependencies",
"dependencies": {
"semver" : "6"
},
"acceptDependencies": {
"semver": "7"
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 38 additions & 0 deletions test/fixtures/accept-dependencies/node_modules/semver/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions test/fixtures/accept-dependencies/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@isaacs/testing-accept-dependencies",
"version": "1.0.0",
"description": "a test of acceptDependencies",
"dependencies": {
"@isaacs/testing-accept-dependencies-dep" : "1",
"semver": "7"
}
}
9 changes: 9 additions & 0 deletions test/fixtures/accept-dependencies/with-newer/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@isaacs/testing-accept-dependencies-with-newer",
"version": "1.0.0",
"description": "a test of acceptDependencies",
"dependencies": {
"@isaacs/testing-accept-dependencies-dep" : "1",
"semver": "7"
}
}

0 comments on commit 705578d

Please sign in to comment.