Revert "install: do not descend into directory deps' child modules"

This reverts commit 45772af

Fix: https://npm.community/t/6-11-1-some-dependencies-are-no-longer-being-installed/9586/4

Also adds 2 tests to verify regression behavior.

PR-URL: #242
Credit: @isaacs
Close: #242
Reviewed-by: @claudiahdz

Author: isaacs

lib/install/inflate-shrinkwrap.js

@@ -50,12 +50,8 @@ function inflateShrinkwrap (topPath, tree, swdeps, opts) {
 
   return BB.each(Object.keys(swdeps), (name) => {
     const sw = swdeps[name]
+    const dependencies = sw.dependencies || {}
     const requested = realizeShrinkwrapSpecifier(name, sw, topPath)
-    // We should not muck about in the node_modules folder of
-    // symlinked packages.  Treat its dependencies as if they
-    // were empty, since it's none of our business.
-    const dependencies = requested.type === 'directory' ? {}
-      : sw.dependencies || {}
     return inflatableChild(
       onDisk[name], name, topPath, tree, sw, requested, opts
     ).then((child) => {
@@ -145,10 +141,6 @@ function isGit (sw) {
 }
 
 function makeFakeChild (name, topPath, tree, sw, requested) {
-  // We should not muck about in the node_modules folder of
-  // symlinked packages.  Treat its dependencies as if they
-  // were empty, since it's none of our business.
-  const isDirectory = requested.type === 'directory'
   const from = sw.from || requested.raw
   const pkg = {
     name: name,
@@ -164,7 +156,7 @@ function makeFakeChild (name, topPath, tree, sw, requested) {
     _spec: requested.rawSpec,
     _where: topPath,
     _args: [[requested.toString(), topPath]],
-    dependencies: isDirectory ? {} : sw.requires
+    dependencies: sw.requires
   }
 
   if (!sw.bundled) {
@@ -175,16 +167,16 @@ function makeFakeChild (name, topPath, tree, sw, requested) {
   }
   const child = createChild({
     package: pkg,
-    loaded: isDirectory,
+    loaded: true,
     parent: tree,
     children: [],
     fromShrinkwrap: requested,
     fakeChild: sw,
     fromBundle: sw.bundled ? tree.fromBundle || tree : null,
     path: childPath(tree.path, pkg),
-    realpath: isDirectory ? requested.fetchSpec : childPath(tree.realpath, pkg),
+    realpath: requested.type === 'directory' ? requested.fetchSpec : childPath(tree.realpath, pkg),
     location: (tree.location === '/' ? '' : tree.location + '/') + pkg.name,
-    isLink: isDirectory,
+    isLink: requested.type === 'directory',
     isInLink: tree.isLink || tree.isInLink,
     swRequires: sw.requires
   })
@@ -203,7 +195,7 @@ function fetchChild (topPath, tree, sw, requested) {
     var isLink = pkg._requested.type === 'directory'
     const child = createChild({
       package: pkg,
-      loaded: isLink,
+      loaded: false,
       parent: tree,
       fromShrinkwrap: requested,
       path: childPath(tree.path, pkg),

test/tap/install-from-local-multipath.js

@@ -167,13 +167,9 @@ test('\'npm install\' should install local packages', function (t) {
       'install', '.'
     ],
     EXEC_OPTS,
-    function (err, code, stdout, stderr) {
+    function (err, code) {
       t.ifError(err, 'error should not exist')
       t.notOk(code, 'npm install exited with code 0')
-      // if the test fails, let's see why
-      if (err || code) {
-        console.error({code, stdout, stderr})
-      }
       t.end()
     }
   )

test/tap/install-link-metadeps-locally.js

@@ -0,0 +1,52 @@
+// XXX Remove in npm v7, when this is no longer how we do things
+const t = require('tap')
+const common = require('../common-tap.js')
+const pkg = common.pkg
+const mkdirp = require('mkdirp')
+const { writeFileSync, statSync } = require('fs')
+const { resolve } = require('path')
+const mr = require('npm-registry-mock')
+const rimraf = require('rimraf')
+
+t.test('setup', t => {
+  mkdirp.sync(resolve(pkg, 'node_modules'))
+  mkdirp.sync(resolve(pkg, 'foo'))
+  writeFileSync(resolve(pkg, 'foo', 'package.json'), JSON.stringify({
+    name: 'foo',
+    version: '1.2.3',
+    dependencies: {
+      underscore: '*'
+    }
+  }))
+
+  writeFileSync(resolve(pkg, 'package.json'), JSON.stringify({
+    name: 'root',
+    version: '1.2.3',
+    dependencies: {
+      foo: 'file:foo'
+    }
+  }))
+
+  mr({ port: common.port }, (er, s) => {
+    if (er) {
+      throw er
+    }
+    t.parent.teardown(() => s.close())
+    t.end()
+  })
+})
+
+t.test('initial install to create package-lock',
+  t => common.npm(['install', '--registry', common.registry], { cwd: pkg })
+    .then(([code]) => t.equal(code, 0, 'command worked')))
+
+t.test('remove node_modules', t =>
+  rimraf(resolve(pkg, 'node_modules'), t.end))
+
+t.test('install again from package-lock', t =>
+  common.npm(['install', '--registry', common.registry], { cwd: pkg })
+    .then(([code]) => {
+      t.equal(code, 0, 'command worked')
+      const underscore = resolve(pkg, 'node_modules', 'underscore')
+      t.equal(statSync(underscore).isDirectory(), true, 'underscore installed')
+    }))

test/tap/install-link-metadeps-subfolders.js

@@ -0,0 +1,68 @@
+const t = require('tap')
+const common = require('../common-tap.js')
+const mkdirp = require('mkdirp')
+const { writeFileSync, readFileSync } = require('fs')
+const { resolve } = require('path')
+const pkg = common.pkg
+const app = resolve(pkg, 'app')
+const lib = resolve(pkg, 'lib')
+const moda = resolve(lib, 'module-a')
+const modb = resolve(lib, 'module-b')
+
+const rimraf = require('rimraf')
+
+t.test('setup', t => {
+  mkdirp.sync(app)
+  mkdirp.sync(moda)
+  mkdirp.sync(modb)
+
+  writeFileSync(resolve(app, 'package.json'), JSON.stringify({
+    name: 'app',
+    version: '1.2.3',
+    dependencies: {
+      moda: 'file:../lib/module-a'
+    }
+  }))
+
+  writeFileSync(resolve(moda, 'package.json'), JSON.stringify({
+    name: 'moda',
+    version: '1.2.3',
+    dependencies: {
+      modb: 'file:../module-b'
+    }
+  }))
+
+  writeFileSync(resolve(modb, 'package.json'), JSON.stringify({
+    name: 'modb',
+    version: '1.2.3'
+  }))
+
+  t.end()
+})
+
+t.test('initial install to create package-lock',
+  t => common.npm(['install'], { cwd: app })
+    .then(([code]) => t.equal(code, 0, 'command worked')))
+
+t.test('remove node_modules', t =>
+  rimraf(resolve(pkg, 'node_modules'), t.end))
+
+t.test('install again from package-lock', t =>
+  common.npm(['install'], { cwd: app })
+    .then(([code]) => {
+      t.equal(code, 0, 'command worked')
+      // verify that module-b is linked under module-a
+      const depPkg = resolve(
+        app,
+        'node_modules',
+        'moda',
+        'node_modules',
+        'modb',
+        'package.json'
+      )
+      const data = JSON.parse(readFileSync(depPkg, 'utf8'))
+      t.strictSame(data, {
+        name: 'modb',
+        version: '1.2.3'
+      })
+    }))