prune trashed optional deps from reified actual tree before returning…

…, but after saving

lib/arborist/reify.js

@@ -26,6 +26,7 @@ const mkdirp = require('mkdirp-infer-owner')
 const rimraf = promisify(require('rimraf'))
 const packageContents = require('@npmcli/installed-package-contents')
 
+const relpath = require('../relpath.js')
 const Diff = require('../diff.js')
 const retirePath = require('../retire-path.js')
 const promiseAllRejectLate = require('promise-all-reject-late')
@@ -68,6 +69,7 @@ const _rollbackRetireShallowNodes = Symbol.for('rollbackRetireShallowNodes')
 const _rollbackCreateSparseTree = Symbol.for('rollbackCreateSparseTree')
 const _rollbackMoveBackRetiredUnchanged = Symbol.for('rollbackMoveBackRetiredUnchanged')
 const _saveIdealTree = Symbol.for('saveIdealTree')
+const _copyIdealToActual = Symbol('copyIdealToActual')
 
 // defined by Ideal mixin
 const _idealTreePrune = Symbol.for('idealTreePrune')
@@ -100,6 +102,7 @@ module.exports = cls => class Reifier extends Ideal(cls) {
       .then(() => this[_runLifecycleScripts]())
       .then(() => this[_removeTrash]())
       .then(() => this[_saveIdealTree](options))
+      .then(() => this[_copyIdealToActual]())
       .then(() => {
         this.finishTracker('reify')
         return this.actualTree
@@ -640,25 +643,23 @@ module.exports = cls => class Reifier extends Ideal(cls) {
   // or shrinkwrap file, and any additions or removals to package.json
   [_saveIdealTree] (options = {}) {
     // the ideal tree is actualized now, hooray!
-    //
-    // XXX this needs to clone rather than copy, so that we can leave failed
-    // optional deps in the ideal tree, but remove them from the actual.
-    // But to do that, we need a way to clone a tree efficiently.
-    this.actualTree = this.idealTree
+    // it still contains all the references to optional nodes that were removed
+    // for install failures.  Those still end up in the shrinkwrap, so we
+    // save it first, then prune out the optional trash, and then return it.
 
     // support save=false option
     if (options.save === false)
       return
 
     if (options.add) {
-      const pkg = this.actualTree.package
+      const pkg = this.idealTree.package
       // need to save these more nicely, now that we know what they are
       for (const [type, specs] of Object.entries(options.add)) {
         if (!specs || typeof specs !== 'object' || Array.isArray(specs))
           continue
 
         for (const [name, spec] of Object.entries(specs)) {
-          const child = this.actualTree.children.get(name)
+          const child = this.idealTree.children.get(name)
           const resolved = child.resolved
           const req = npa(spec)
           const res = npa(resolved)
@@ -676,17 +677,28 @@ module.exports = cls => class Reifier extends Ideal(cls) {
         }
       }
       // refresh the edges so they have the correct specs
-      this.actualTree.package = pkg
+      this.idealTree.package = pkg
     }
 
     // XXX preserve indentation maybe?
-    const pj = resolve(this.actualTree.path, 'package.json')
+    const pj = resolve(this.idealTree.path, 'package.json')
     return Promise.all([
-      this.actualTree.meta.save(),
+      this.idealTree.meta.save(),
       writeFile(pj, JSON.stringify({
-        ...this.actualTree.package,
+        ...this.idealTree.package,
         _id: undefined,
       }, null, 2) + '\n'),
     ])
   }
+
+  [_copyIdealToActual] () {
+    this.actualTree = this.idealTree
+    this.idealTree = null
+    for (const path of this[_trashList]) {
+      const loc = relpath(this.path, path)
+      const node = this.actualTree.inventory.get(loc)
+      if (node && node.root === this.actualTree)
+        node.parent = null
+    }
+  }
 }