diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index be920272d48f0..9a98c26be47bb 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -1295,7 +1295,8 @@ module.exports = cls => class Reifier extends cls { const updatedTrees = new Set() const updateNodes = nodes => { - for (const { name, tree: addTree } of nodes) { + for (const node of nodes) { + const { name, tree: addTree } = node // addTree either the root, or a workspace const edge = addTree.edgesOut.get(name) const pkg = addTree.package @@ -1371,6 +1372,10 @@ module.exports = cls => class Reifier extends cls { newSpec = req.saveSpec } + // delete optional dep from prod if installation doesn't fail + const shouldDeleteFromProd = pkg.optionalDependencies && pkg.optionalDependencies[name] && pkg.dependencies + && !this[_trashList].has(addTree.children.get(name).path) + if (options.saveType) { const depType = saveTypeMap.get(options.saveType) pkg[depType][name] = newSpec @@ -1378,8 +1383,14 @@ module.exports = cls => class Reifier extends cls { // if it is empty it will be deleted later if (options.saveType === 'prod' && pkg.optionalDependencies) { delete pkg.optionalDependencies[name] + } else if (options.saveType === 'optional' && shouldDeleteFromProd) { + delete pkg.dependencies[name] } } else { + if (shouldDeleteFromProd) { + delete pkg.dependencies[name] + } + if (hasSubKey(pkg, 'dependencies', name)) { pkg.dependencies[name] = newSpec } diff --git a/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs index ffe63992d8c34..d928fb0b4d578 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs @@ -33087,6 +33087,55 @@ exports[`test/arborist/reify.js TAP save proper lockfile with bins when upgradin exports[`test/arborist/reify.js TAP save-prod, with optional > must match snapshot 1`] = ` {"dependencies":{"abbrev":"^1.1.1"}} +` +exports[`test/arborist/reify.js TAP delete from prod, if optional dep is added > must match snapshot 1`] = ` +{ + "name": "tap-testdir-reify-delete-from-prod-if-optional-dep-is-added", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "optionalDependencies": { + "abbrev": "^1.1.1" + } + }, + "node_modules/abbrev": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==", + "license": "ISC", + "optional": true + } + } +} + +` + +exports[`test/arborist/reify.js TAP keep in prod, if optional dep fails install > must match snapshot 1`] = ` +{ + "name": "tap-testdir-reify-keep-in-prod-if-optional-dep-fails-install", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "dependencies": { + "@isaacs/testing-fail-install": "*" + }, + "optionalDependencies": { + "@isaacs/testing-fail-install": "^1.0.0" + } + }, + "node_modules/@isaacs/testing-fail-install": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/@isaacs/testing-fail-install/-/testing-fail-install-1.0.0.tgz", + "integrity": "sha512-MlK05L4wasrk2/fQqgCeR8s+1KE8LEfMI3ZzaGfR5Dyvgi4R8vQY0fvwYcvKh6cqrmMX7wORvGKi5YJn8LZ9bg==", + "hasInstallScript": true, + "license": "ISC", + "optional": true + } + } +} + ` exports[`test/arborist/reify.js TAP saveBundle > must match snapshot 1`] = ` diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index 0a7fb416040c0..7b0c1f2718a94 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -1857,6 +1857,36 @@ t.test('save-prod, with optional', async t => { t.matchSnapshot(fs.readFileSync(path + '/package.json', 'utf8')) }) +t.test('delete from prod, if optional dep is added', async t => { + // add to existing optional dep in lockfile + const pathNoSaveType = t.testdir({ + 'package.json': JSON.stringify({ + optionalDependencies: { abbrev: '*' }, + }), + }) + const arbNoSaveType = newArb({ path: pathNoSaveType }) + await arbNoSaveType.reify({ add: ['abbrev'] }) + const resultNoSaveType = fs.readFileSync(pathNoSaveType + '/package-lock.json', 'utf8') + + // add to empty lockfile using optional saveType + const pathSaveType = t.testdir({ 'package.json': '{}' }) + const arbSaveType = newArb({ path: pathSaveType }) + await arbSaveType.reify({ add: ['abbrev'], saveType: 'optional' }) + const resultSaveType = fs.readFileSync(pathSaveType + '/package-lock.json', 'utf8') + + // both lockfiles should be the same + t.same(resultNoSaveType, resultSaveType) + t.matchSnapshot(resultNoSaveType) +}) + +t.test('keep in prod, if optional dep fails install', async t => { + const path = t.testdir({ 'package.json': '{}' }) + const arb = newArb({ path }) + + await arb.reify({ add: ['@isaacs/testing-fail-install'], saveType: 'optional' }) + t.matchSnapshot(fs.readFileSync(path + '/package-lock.json', 'utf8')) +}) + t.test('saveBundle', async t => { const path = t.testdir({ 'package.json': JSON.stringify({