Skip to content

Commit

Permalink
Update resolver to use empty module if redirected against complete pa…
Browse files Browse the repository at this point in the history
…th 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
  • Loading branch information
huntie authored and facebook-github-bot committed Mar 1, 2023
1 parent c0da607 commit 0f1846a
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 25 deletions.
6 changes: 5 additions & 1 deletion packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
41 changes: 17 additions & 24 deletions packages/metro/src/DeltaBundler/__tests__/resolver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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': '',
},
},
});

Expand All @@ -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 () => {
Expand All @@ -1156,7 +1155,6 @@ type MockFSDirContents = $ReadOnly<{
'package.json': JSON.stringify({
name: 'aPackage',
[(browserField: string)]: {
'left-pad': false,
'./foo.js': false,
},
}),
Expand All @@ -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'),
});
});
});
});
Expand Down

0 comments on commit 0f1846a

Please sign in to comment.