From 0f1846a64d7ec27ad7c42b1aee2a049e0fad9a5a Mon Sep 17 00:00:00 2001
From: Alex Hunt <alexeh@meta.com>
Date: Wed, 1 Mar 2023 06:52:16 -0800
Subject: [PATCH] Update resolver to use empty module if redirected against
 complete path in node_modules

Summary:
Removes undefined behaviour in `metro-resolver` caused by type suppression against `redirectModulePath` results inside `nodeModulesPaths` expansion.

Changelog: **[Fix]** Returning `false` from `context.redirectModulePath` will resolve to empty module in all cases

Reviewed By: motiz88

Differential Revision: D43665372

fbshipit-source-id: 3a70aa4e9639ec8ad5378b8a53f2d0ff5d76e09d
---
 packages/metro-resolver/src/resolve.js        |  6 ++-
 .../DeltaBundler/__tests__/resolver-test.js   | 41 ++++++++-----------
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js
index ce2be4dc62..c64a85018b 100644
--- a/packages/metro-resolver/src/resolve.js
+++ b/packages/metro-resolver/src/resolve.js
@@ -126,7 +126,11 @@ function resolve(
     .concat(extraPaths);
   for (let i = 0; i < allDirPaths.length; ++i) {
     const candidate = context.redirectModulePath(allDirPaths[i]);
-    // $FlowFixMe[incompatible-call]
+
+    if (candidate === false) {
+      return {type: 'empty'};
+    }
+
     const result = resolvePackage(context, candidate, platform);
     if (result.type === 'resolved') {
       return result.resolution;
diff --git a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js
index c0dd6f11a3..33257f7c8b 100644
--- a/packages/metro/src/DeltaBundler/__tests__/resolver-test.js
+++ b/packages/metro/src/DeltaBundler/__tests__/resolver-test.js
@@ -1107,11 +1107,17 @@ type MockFSDirContents = $ReadOnly<{
                     name: 'aPackage',
                     [(browserField: string)]: {
                       'left-pad': false,
-                      './foo.js': false,
                     },
                   }),
                   'index.js': '',
                 },
+                'left-pad': {
+                  'package.json': JSON.stringify({
+                    name: 'left-pad',
+                  }),
+                  'index.js': '',
+                  'foo.js': '',
+                },
               },
             });
 
@@ -1128,23 +1134,16 @@ type MockFSDirContents = $ReadOnly<{
               type: 'sourceFile',
               filePath: p('/root/emptyModule.js'),
             });
+            // Existing limitation: Subpaths of a package are not redirected by "browser"
             expect(
               resolver.resolve(
                 p('/root/node_modules/aPackage/index.js'),
-                './foo',
+                'left-pad/foo',
               ),
             ).toEqual({
               type: 'sourceFile',
-              filePath: p('/root/emptyModule.js'),
+              filePath: p('/root/node_modules/left-pad/foo.js'),
             });
-
-            // TODO: Are the following two cases expected behaviour?
-            expect(() =>
-              resolver.resolve(p('/root/index.js'), 'aPackage/foo'),
-            ).toThrow();
-            expect(() =>
-              resolver.resolve(p('/root/index.js'), 'aPackage/foo.js'),
-            ).toThrow();
           });
 
           it('supports excluding a package when the empty module is a relative path', async () => {
@@ -1156,7 +1155,6 @@ type MockFSDirContents = $ReadOnly<{
                   'package.json': JSON.stringify({
                     name: 'aPackage',
                     [(browserField: string)]: {
-                      'left-pad': false,
                       './foo.js': false,
                     },
                   }),
@@ -1172,29 +1170,24 @@ type MockFSDirContents = $ReadOnly<{
             expect(
               resolver.resolve(
                 p('/root/node_modules/aPackage/index.js'),
-                'left-pad',
+                './foo',
               ),
             ).toEqual({
               type: 'sourceFile',
               filePath: p('/root/emptyModule.js'),
             });
             expect(
-              resolver.resolve(
-                p('/root/node_modules/aPackage/index.js'),
-                './foo',
-              ),
+              resolver.resolve(p('/root/index.js'), 'aPackage/foo'),
             ).toEqual({
               type: 'sourceFile',
               filePath: p('/root/emptyModule.js'),
             });
-
-            // TODO: Are the following two cases expected behaviour?
-            expect(() =>
-              resolver.resolve(p('/root/index.js'), 'aPackage/foo'),
-            ).toThrow();
-            expect(() =>
+            expect(
               resolver.resolve(p('/root/index.js'), 'aPackage/foo.js'),
-            ).toThrow();
+            ).toEqual({
+              type: 'sourceFile',
+              filePath: p('/root/emptyModule.js'),
+            });
           });
         });
       });