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

PR-URL: #235
Credit: @isaacs
Close: #235
Reviewed-by: @nlf
  • Loading branch information
@isaacs
isaacs committed Feb 18, 2021
1 parent 7e8b824 commit b0997c9
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 39 deletions.
7 changes: 5 additions & 2 deletions 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,8 @@ class Node {
this[_loadDepType](peerDependencies, 'peer')
this[_loadDepType](peerOptional, 'peerOptional')
}

this[_loadDepType](this.package.dependencies, 'prod')
}

[_loadDepType] (obj, type) {
Expand All @@ -763,8 +764,10 @@ class Node {
for (const [name, spec] of Object.entries(obj || {})) {
const accept = ad[name]
// if it's already set, then we keep the existing edge
// Prod deps should not be marked as dev, however.
// NB: the Edge ctor adds itself to from.edgesOut
if (!this.edgesOut.get(name))
const current = this.edgesOut.get(name)
if (!current || current.dev && type === 'prod')
new Edge({ from, name, spec, accept, 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
Loading

0 comments on commit b0997c9

Please sign in to comment.