From 918957fd564539014dc9b4a404394a62905fff68 Mon Sep 17 00:00:00 2001
From: Sharmila Jesupaul <sharmilajesupaul@users.noreply.github.com>
Date: Thu, 11 Nov 2021 10:13:09 -0800
Subject: [PATCH] Respect transitive deps when using `nodeModulesPaths` (#738)

Summary:
**Summary**
This fixes https://github.com/facebook/metro/issues/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: https://github.com/facebook/metro/pull/738

Reviewed By: motiz88

Differential Revision: D32310356

Pulled By: GijsWeterings

fbshipit-source-id: 158e401cb47cc4a4bcc11687ef7e34caa8ce3dde
---
 .../src/__tests__/index-test.js               | 33 ++++++++++++++++++-
 packages/metro-resolver/src/resolve.js        |  6 +++-
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/packages/metro-resolver/src/__tests__/index-test.js b/packages/metro-resolver/src/__tests__/index-test.js
index 2fb86f0d72..1aa91f4c32 100644
--- a/packages/metro-resolver/src/__tests__/index-test.js
+++ b/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,
diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js
index 0dd56811fd..70109de178 100644
--- a/packages/metro-resolver/src/resolve.js
+++ b/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) {