From 02fade69cad95488e1afba0c3dba8e25d8453bc5 Mon Sep 17 00:00:00 2001
From: Sharmila Jesupaul <sharmila.jesupaul@airbnb.com>
Date: Tue, 9 Nov 2021 16:57:50 -0800
Subject: [PATCH 1/2] [metro-resolver] Respect transitive deps when using
 `nodeModulesPaths`

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.

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 `banan-module` dependency first.
---
 .../src/__tests__/index-test.js               | 33 ++++++++++++++++++-
 packages/metro-resolver/src/resolve.js        |  5 ++-
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/packages/metro-resolver/src/__tests__/index-test.js b/packages/metro-resolver/src/__tests__/index-test.js
index 8390018860..f58d1c4763 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,
+              },
+            },
           },
         },
       },
@@ -216,14 +226,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 5d660a5a77..c51d60a3a5 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,8 @@ function resolve(
     } while (candidate !== next);
   }
 
+  nodeModulesPaths.push(...context.nodeModulesPaths);
+
   const extraPaths = [];
   const {extraNodeModules} = context;
   if (extraNodeModules) {

From 2282b7f89956004ac518d905562d05d4b15206af Mon Sep 17 00:00:00 2001
From: Sharmila Jesupaul <sharmilajesupaul@users.noreply.github.com>
Date: Wed, 10 Nov 2021 14:31:18 -0800
Subject: [PATCH 2/2] Add a comment explaining a changed line

Co-authored-by: Rae Liu <raejin@users.noreply.github.com>
---
 packages/metro-resolver/src/resolve.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js
index c51d60a3a5..51ef4b2a71 100644
--- a/packages/metro-resolver/src/resolve.js
+++ b/packages/metro-resolver/src/resolve.js
@@ -103,6 +103,7 @@ function resolve(
     } while (candidate !== next);
   }
 
+  // Fall back to `nodeModulesPaths` after hierarchical lookup, similar to $NODE_PATH
   nodeModulesPaths.push(...context.nodeModulesPaths);
 
   const extraPaths = [];