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

Commit

Permalink
Prefer peer over prod dep, if specified in both
Browse files Browse the repository at this point in the history 
Also, preserve the duplication if we update or modify the package.json
file.

Fix: npm/rfcs#324
  • Loading branch information
@isaacs
isaacs committed Feb 17, 2021
1 parent 9c6cd63 commit 718e8e5
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 38 deletions.
2 changes: 1 addition & 1 deletion lib/node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,6 @@ class Node {
// Note the subtle breaking change from v6: it is no longer possible
// to have a different spec for a devDep than production dep.
this[_loadDepType](this.package.optionalDependencies, 'optional')
this[_loadDepType](this.package.dependencies, 'prod')

// Linked targets that are disconnected from the tree are tops,
// but don't have a 'path' field, only a 'realpath', because we
Expand All @@ -755,6 +754,7 @@ class Node {
this[_loadDepType](peerDependencies, 'peer')
this[_loadDepType](peerOptional, 'peerOptional')
}
this[_loadDepType](this.package.dependencies, 'prod')
}

[_loadDepType] (obj, type) {
Expand Down
19 changes: 18 additions & 1 deletion lib/update-root-package-json.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,29 @@ const updateRootPackageJson = async tree => {
}

// if there's no package.json, just use internal pkg info as source of truth
const packageJsonContent = originalContent || depsData
// clone the object though, so we can still refer to what it originally was
const packageJsonContent = !originalContent ? depsData
: Object.assign({}, originalContent)

// loop through all types of dependencies and update package json content
for (const type of depTypes)
packageJsonContent[type] = depsData[type]

// if original package.json had dep in peerDeps AND deps, preserve that.
const { dependencies: origProd, peerDependencies: origPeer } =
originalContent || {}
const { peerDependencies: newPeer } = packageJsonContent
if (origProd && origPeer && newPeer) {
// we have original prod/peer deps, and new peer deps
// copy over any that were in both in the original
for (const name of Object.keys(origPeer)) {
if (origProd[name] !== undefined && newPeer[name] !== undefined) {
packageJsonContent.dependencies = packageJsonContent.dependencies || {}
packageJsonContent.dependencies[name] = newPeer[name]
}
}
}

// format content
const {
[Symbol.for('indent')]: indent,
Expand Down
60 changes: 30 additions & 30 deletions tap-snapshots/test-node.js-TAP.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -670,8 +670,8 @@ exports[`test/node.js TAP testing with dep tree with meta > add new meta under p
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -917,10 +917,10 @@ exports[`test/node.js TAP testing with dep tree with meta > add new meta under p
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -993,8 +993,8 @@ exports[`test/node.js TAP testing with dep tree with meta > add new meta under p
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -1421,8 +1421,8 @@ exports[`test/node.js TAP testing with dep tree with meta > initial load with so
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -1615,10 +1615,10 @@ exports[`test/node.js TAP testing with dep tree with meta > initial load with so
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -1667,8 +1667,8 @@ exports[`test/node.js TAP testing with dep tree with meta > initial load with so
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -1964,8 +1964,8 @@ exports[`test/node.js TAP testing with dep tree with meta > move meta to top lev
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -2189,10 +2189,10 @@ exports[`test/node.js TAP testing with dep tree with meta > move meta to top lev
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand All @@ -2210,8 +2210,8 @@ exports[`test/node.js TAP testing with dep tree with meta > move meta to top lev
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -2508,8 +2508,8 @@ exports[`test/node.js TAP testing with dep tree with meta > move new meta to top
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -2780,10 +2780,10 @@ exports[`test/node.js TAP testing with dep tree with meta > move new meta to top
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand All @@ -2801,8 +2801,8 @@ exports[`test/node.js TAP testing with dep tree with meta > move new meta to top
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -3169,8 +3169,8 @@ exports[`test/node.js TAP testing with dep tree with meta > move new meta to top
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -3441,10 +3441,10 @@ exports[`test/node.js TAP testing with dep tree with meta > move new meta to top
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand All @@ -3462,8 +3462,8 @@ exports[`test/node.js TAP testing with dep tree with meta > move new meta to top
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -3885,8 +3885,8 @@ exports[`test/node.js TAP testing with dep tree without meta > add new meta unde
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -4132,10 +4132,10 @@ exports[`test/node.js TAP testing with dep tree without meta > add new meta unde
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -4208,8 +4208,8 @@ exports[`test/node.js TAP testing with dep tree without meta > add new meta unde
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -4636,8 +4636,8 @@ exports[`test/node.js TAP testing with dep tree without meta > initial load with
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -4830,10 +4830,10 @@ exports[`test/node.js TAP testing with dep tree without meta > initial load with
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -4882,8 +4882,8 @@ exports[`test/node.js TAP testing with dep tree without meta > initial load with
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -5179,8 +5179,8 @@ exports[`test/node.js TAP testing with dep tree without meta > move meta to top
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -5404,10 +5404,10 @@ exports[`test/node.js TAP testing with dep tree without meta > move meta to top
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand All @@ -5425,8 +5425,8 @@ exports[`test/node.js TAP testing with dep tree without meta > move meta to top
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -5723,8 +5723,8 @@ exports[`test/node.js TAP testing with dep tree without meta > move new meta to
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -5995,10 +5995,10 @@ exports[`test/node.js TAP testing with dep tree without meta > move new meta to
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand All @@ -6016,8 +6016,8 @@ exports[`test/node.js TAP testing with dep tree without meta > move new meta to
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -6384,8 +6384,8 @@ exports[`test/node.js TAP testing with dep tree without meta > move new meta to
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down Expand Up @@ -6656,10 +6656,10 @@ exports[`test/node.js TAP testing with dep tree without meta > move new meta to
"optional" => Edge {},
"overlap" => Edge {},
"optMissing" => Edge {},
"dev" => Edge {},
"prod" => Edge {},
"bundled" => Edge {},
"missing" => Edge {},
"dev" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand All @@ -6677,8 +6677,8 @@ exports[`test/node.js TAP testing with dep tree without meta > move new meta to
Edge {},
},
"edgesOut": Map {
"meta" => Edge {},
"peer" => Edge {},
"meta" => Edge {},
},
"errors": Array [],
"extraneous": true,
Expand Down
14 changes: 14 additions & 0 deletions test/node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2181,3 +2181,17 @@ t.test('globaTop set for children of global link root target', async t => {
})
t.equal(gtop.globalTop, true)
})

t.test('prefer peer over prod', async t => {
const n = new Node({
pkg: {
dependencies: {
foo: '1.x',
},
peerDependencies: {
foo: '1.x',
},
},
})
t.equal(n.edgesOut.get('foo').type, 'peer', 'prefer peer over prod dep')
})
Loading

0 comments on commit 718e8e5

Please sign in to comment.