Skip to content

Commit

Permalink
[metro-resolver] Respect transitive deps when using nodeModulesPaths
Browse files Browse the repository at this point in the history
This fixes #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.
  • Loading branch information
sharmilajesupaul committed Nov 10, 2021
1 parent 257dcb5 commit 02fade6
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
33 changes: 32 additions & 1 deletion packages/metro-resolver/src/__tests__/index-test.js
Expand Up @@ -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,
},
},
},
},
},
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion packages/metro-resolver/src/resolve.js
Expand Up @@ -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 {
Expand All @@ -102,6 +103,8 @@ function resolve(
} while (candidate !== next);
}

nodeModulesPaths.push(...context.nodeModulesPaths);

const extraPaths = [];
const {extraNodeModules} = context;
if (extraNodeModules) {
Expand Down

0 comments on commit 02fade6

Please sign in to comment.