fix: do not run a double install with --shamefully-flatten (#46)

* fix: do not run a double install with --shamefully-flatten

* fix: reinstallForFlatten forces flatten even if no changes are seen

src/api/install.ts

@@ -204,12 +204,6 @@ export async function install (maybeOpts: InstallOptions) {
 
     await installInContext(installType, specs, [], ctx, preferredVersions, opts)
 
-    if (opts.shamefullyFlatten && specs.length > 0) {
-      await installPkgs(specs.map(spec => spec.raw), Object.assign({},
-        opts, {lock: false, reinstallForFlatten: true, update: false}
-      ))
-    }
-
     if (scripts['install']) {
       await npmRunScript('install', ctx.pkg, scriptsOpts)
     }
@@ -391,6 +385,7 @@ async function installInContext (
     hasManifestInShrinkwrap,
     sideEffectsCache: opts.sideEffectsCache,
     reinstallForFlatten: opts.reinstallForFlatten,
+    shamefullyFlatten: opts.shamefullyFlatten,
   }
   const nonLinkedPkgs = await pFilter(packagesToInstall,
     async (wantedDependency: WantedDependency) => {
@@ -552,6 +547,7 @@ async function installInContext (
     outdatedPkgs: installCtx.outdatedPkgs,
     sideEffectsCache: opts.sideEffectsCache,
     shamefullyFlatten: opts.shamefullyFlatten,
+    reinstallForFlatten: Boolean(opts.reinstallForFlatten),
   })
 
   ctx.pendingBuilds = ctx.pendingBuilds

src/link/index.ts

@@ -44,6 +44,7 @@ export default async function linkPackages (
     outdatedPkgs: {[pkgId: string]: string},
     sideEffectsCache: boolean,
     shamefullyFlatten: boolean,
+    reinstallForFlatten: boolean,
   }
 ): Promise<{
   linkedPkgsMap: DependencyTreeNodeMap,
@@ -126,10 +127,6 @@ export default async function linkPackages (
     })
   }
 
-  if (opts.shamefullyFlatten) {
-    await shamefullyFlattenTree(flatResolvedDeps, opts)
-  }
-
   if (!opts.dryRun) {
     await linkBins(opts.baseNodeModules, opts.bin)
   }
@@ -159,6 +156,10 @@ export default async function linkPackages (
     currentShrinkwrap = newCurrentShrinkwrap
   }
 
+  if (opts.shamefullyFlatten && (opts.reinstallForFlatten || newDepPaths.length > 0 || removedPkgIds.size > 0)) {
+    await shamefullyFlattenTree(flatResolvedDeps, currentShrinkwrap, opts)
+  }
+
   return {
     linkedPkgsMap: pkgsToLink,
     wantedShrinkwrap: newShr,
@@ -170,6 +171,7 @@ export default async function linkPackages (
 
 async function shamefullyFlattenTree(
   flatResolvedDeps: DependencyTreeNode[],
+  currentShrinkwrap: Shrinkwrap,
   opts: {
     force: boolean,
     dryRun: boolean,
@@ -177,12 +179,11 @@ async function shamefullyFlattenTree(
     bin: string,
     pkg: PackageJson,
     outdatedPkgs: {[pkgId: string]: string},
-    wantedShrinkwrap: Shrinkwrap,
   },
 ) {
   const pkgNamesExcludedFromFlattening = {}
   // first of all, exclude the root packages, as they are already linked
-  for (let name of R.keys(opts.wantedShrinkwrap.specifiers)) {
+  for (let name of R.keys(currentShrinkwrap.specifiers)) {
     pkgNamesExcludedFromFlattening[name] = true
   }
 

src/resolveDependencies.ts

@@ -95,6 +95,7 @@ export default async function resolveDependencies (
     hasManifestInShrinkwrap: boolean,
     sideEffectsCache: boolean,
     reinstallForFlatten?: boolean,
+    shamefullyFlatten?: boolean,
   }
 ): Promise<PkgAddress[]> {
   const resolvedDependencies = options.resolvedDependencies || {}
@@ -130,6 +131,7 @@ export default async function resolveDependencies (
             update,
             proceed,
             reinstallForFlatten: options.reinstallForFlatten,
+            shamefullyFlatten: options.shamefullyFlatten,
             sideEffectsCache: options.sideEffectsCache,
             ...getInfoFromShrinkwrap(ctx.wantedShrinkwrap, reference, wantedDependency.alias, ctx.registry),
           })
@@ -246,13 +248,14 @@ async function install (
     hasManifestInShrinkwrap: boolean,
     sideEffectsCache: boolean,
     reinstallForFlatten?: boolean,
+    shamefullyFlatten?: boolean,
   }
 ): Promise<PkgAddress | null> {
   const keypath = options.keypath || []
   const proceed = options.proceed || !options.shrinkwrapResolution || ctx.force || keypath.length <= ctx.depth
   const parentIsInstallable = options.parentIsInstallable === undefined || options.parentIsInstallable
 
-  if (!options.reinstallForFlatten && !proceed && options.depPath &&
+  if (!options.shamefullyFlatten && !options.reinstallForFlatten && !proceed && options.depPath &&
     // if package is not in `node_modules/.shrinkwrap.yaml`
     // we can safely assume that it doesn't exist in `node_modules`
     options.relDepPath && ctx.currentShrinkwrap.packages && ctx.currentShrinkwrap.packages[options.relDepPath] &&
@@ -452,6 +455,7 @@ async function install (
         useManifestInfoFromShrinkwrap,
         sideEffectsCache: options.sideEffectsCache,
         reinstallForFlatten: options.reinstallForFlatten,
+        shamefullyFlatten: options.shamefullyFlatten,
       }
     )
     ctx.childrenByParentId[pkgResponse.body.id] = children.map(child => ({
@@ -538,6 +542,7 @@ async function resolveDependenciesOfPackage (
     useManifestInfoFromShrinkwrap: boolean,
     sideEffectsCache: boolean,
     reinstallForFlatten?: boolean,
+    shamefullyFlatten?: boolean,
   }
 ): Promise<PkgAddress[]> {