Skip to content

Commit

Permalink
fix: package.json saving optional deps
Browse files Browse the repository at this point in the history
When saving over existing files that contains optionalDependencies it
should avoid also saving that dep to dependencies, respecting our docs
recommendation on it:

    Entries in optionalDependencies will override entries of the same
    name in dependencies, so it's usually best to only put in one place.

This patches this UX problem by adding an extra check that will avoid
adding a dependency to the package.json `dependencies` object in case
that package is already listed under `optionalDependencies`.

Relates to: npm/arborist#221
Fixes: npm/cli#724

PR-URL: #109
Credit: @ruyadorno
Close: #109
Reviewed-by: @nlf, @wraithgar
  • Loading branch information
ruyadorno committed Feb 4, 2021
1 parent bbc479c commit da71c05
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
16 changes: 15 additions & 1 deletion init-package-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function init (dir, input, config, cb) {
if (!pkg.description)
pkg.description = data.description

var d = JSON.stringify(pkg, null, 2) + '\n'
var d = JSON.stringify(updateDeps(pkg), null, 2) + '\n'
function write (yes) {
fs.writeFile(packageFile, d, 'utf8', function (er) {
if (!er && yes && !config.get('silent')) {
Expand Down Expand Up @@ -132,6 +132,20 @@ function init (dir, input, config, cb) {

}

function updateDeps(depsData) {
// optionalDependencies don't need to be repeated in two places
if (depsData.dependencies) {
if (depsData.optionalDependencies) {
for (const name of Object.keys(depsData.optionalDependencies))
delete depsData.dependencies[name]
}
if (Object.keys(depsData.dependencies).length === 0)
delete depsData.dependencies
}

return depsData
}

// turn the objects into somewhat more humane strings.
function unParsePeople (data) {
if (data.author) data.author = unParsePerson(data.author)
Expand Down
7 changes: 7 additions & 0 deletions test/dependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ var EXPECT = {
},
devDependencies: {
'mocha': '^1.0.0'
},
optionalDependencies: {
'abbrev': '*'
}
}

Expand All @@ -29,7 +32,11 @@ process.chdir(testdir)

fs.writeFileSync(path.resolve(testdir, 'package.json'), JSON.stringify({
dependencies: {
'abbrev': '*',
'tap': '*'
},
optionalDependencies: {
'abbrev': '*'
}
}))

Expand Down

0 comments on commit da71c05

Please sign in to comment.