Respect transitive deps when using nodeModulesPaths (facebook#738)

Summary:
**Summary**
This fixes facebook#737 take a look at the issue for more info. I've also added a test case to hopefully catch this in the future. The order of the paths is important in this change.

This change changes the resolution order slightly, we can see this by observing `allDirPaths` https://github.com/facebook/metro/blob/02fade69cad95488e1afba0c3dba8e25d8453bc5/packages/metro-resolver/src/resolve.js#L126-L128

Without this change, the `allDirPaths` looks like this:
```
    [
      '/other-root/node_modules/banana-module',
      '/other-root/node_modules/banana/node_modules/banana-module',
      '/other-root/node_modules/node_modules/banana-module',
      '/other-root/node_modules/banana-module',
      '/node_modules/banana-module'
    ]
```

See how the `banana-module` nested inside `banana` is second & not first.

With this change `allDirPaths` is update to this:
```
    [
      '/other-root/node_modules/banana/node_modules/banana-module',
      '/other-root/node_modules/node_modules/banana-module',
      '/other-root/node_modules/banana-module',
      '/node_modules/banana-module',
      '/other-root/node_modules/banana-module'
    ]
```

We correctly attempt to resolve the transitive `banana-module` dependency first.

**Test plan**
Added a unit test

Pull Request resolved: facebook#738

Reviewed By: motiz88

Differential Revision: D32310356

Pulled By: GijsWeterings

fbshipit-source-id: 158e401cb47cc4a4bcc11687ef7e34caa8ce3dde

packages/metro-resolver/src/__tests__/index-test.js

@@ -56,9 +56,19 @@ const CONTEXT: ResolutionContext = (() => {
       },
       'other-root': {
         node_modules: {
+          'banana-module': {
+            'package.json': true,
+            'main.js': true,
+          },
           banana: {
             'package.json': true,
             'main.js': true,
+            node_modules: {
+              'banana-module': {
+                'package.json': true,
+                'main.js': true,
+              },
+            },
           },
         },
       },
@@ -217,14 +227,35 @@ it('uses `nodeModulesPaths` to find additional node_modules not in the direct pa
   expect(() => Resolver.resolve(context, 'kiwi', null))
     .toThrowErrorMatchingInlineSnapshot(`
     "Module does not exist in the Haste module map or in these directories:
-      /other-root/node_modules
       /root/project/node_modules
       /root/node_modules
       /node_modules
+      /other-root/node_modules
     "
   `);
 });
 
+it('resolves transitive dependencies when using `nodeModulesPaths`', () => {
+  const context = Object.assign(
+    {},
+    {...CONTEXT, originModulePath: '/other-root/node_modules/banana/main.js'},
+    {
+      nodeModulesPaths: ['/other-root/node_modules'],
+    },
+  );
+
+  expect(Resolver.resolve(context, 'banana-module', null)).toEqual({
+    type: 'sourceFile',
+    filePath:
+      '/other-root/node_modules/banana/node_modules/banana-module/main.js',
+  });
+
+  expect(Resolver.resolve(context, 'banana-module', null)).not.toEqual({
+    type: 'sourceFile',
+    filePath: '/other-root/node_modules/banana-module/main.js',
+  });
+});
+
 describe('disableHierarchicalLookup', () => {
   const context = Object.assign({}, CONTEXT, {
     disableHierarchicalLookup: true,

packages/metro-resolver/src/resolve.js

@@ -90,9 +90,10 @@ function resolve(
     } catch (error) {}
   }
 
-  const nodeModulesPaths = Array.from(context.nodeModulesPaths);
+  const nodeModulesPaths = [];
   let next = path.dirname(originModulePath);
   const {disableHierarchicalLookup} = context;
+
   if (!disableHierarchicalLookup) {
     let candidate;
     do {
@@ -102,6 +103,9 @@ function resolve(
     } while (candidate !== next);
   }
 
+  // Fall back to `nodeModulesPaths` after hierarchical lookup, similar to $NODE_PATH
+  nodeModulesPaths.push(...context.nodeModulesPaths);
+
   const extraPaths = [];
   const {extraNodeModules} = context;
   if (extraNodeModules) {