Skip to content

Commit

Permalink
Update exports resolution to ignore absolute and relative imports
Browse files Browse the repository at this point in the history
Summary: Changelog: **[Experimental]** Fix bug where `"exports"` field would be used on relative imports within a package

Reviewed By: motiz88

Differential Revision: D43665370

fbshipit-source-id: dfed117b0f6d6e8bed1469cb2ece0f241cf079e7
  • Loading branch information
huntie authored and facebook-github-bot committed Mar 2, 2023
1 parent 7e92227 commit cd25c2b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 6 deletions.
30 changes: 30 additions & 0 deletions packages/metro-resolver/src/__tests__/package-exports-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ describe('with package exports resolution enabled', () => {
'.': './index.js',
'./foo.js': './lib/foo.js',
'./baz': './node_modules/baz/index.js',
'./metadata.json': './metadata.min.json',
},
}),
'/root/node_modules/test-pkg/index.js': '',
Expand All @@ -253,6 +254,8 @@ describe('with package exports resolution enabled', () => {
'/root/node_modules/test-pkg/lib/foo.ios.js': '',
'/root/node_modules/test-pkg/private/bar.js': '',
'/root/node_modules/test-pkg/node_modules/baz/index.js': '',
'/root/node_modules/test-pkg/metadata.json': '',
'/root/node_modules/test-pkg/metadata.min.json': '',
}),
originModulePath: '/root/src/main.js',
unstable_enablePackageExports: true,
Expand Down Expand Up @@ -305,6 +308,31 @@ describe('with package exports resolution enabled', () => {
`);
});

test('should not use "exports" for internal relative imports within a package', () => {
const context = {
...baseContext,
originModulePath: '/root/node_modules/test-pkg/lib/foo.js',
};

expect(Resolver.resolve(context, '../metadata.json', null)).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/metadata.json',
});
});

test('should not use "exports" for an absolute import path', () => {
expect(
Resolver.resolve(
baseContext,
'/root/node_modules/test-pkg/metadata.json',
null,
),
).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/metadata.json',
});
});

describe('should resolve "exports" target directly', () => {
test('without expanding `sourceExts`', () => {
expect(Resolver.resolve(baseContext, 'test-pkg/foo.js', null)).toEqual({
Expand Down Expand Up @@ -374,13 +402,15 @@ describe('with package exports resolution enabled', () => {
name: 'test-pkg',
main: 'index.js',
exports: {
'.': './src/index.js',
'./features/*.js': './src/features/*.js',
'./features/bar/*.js': {
'react-native': null,
},
'./assets/*': './assets/*',
},
}),
'/root/node_modules/test-pkg/src/index.js': '',
'/root/node_modules/test-pkg/src/features/foo.js': '',
'/root/node_modules/test-pkg/src/features/foo.js.js': '',
'/root/node_modules/test-pkg/src/features/bar/Bar.js': '',
Expand Down
13 changes: 7 additions & 6 deletions packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function resolve(
}

if (isRelativeImport(moduleName) || path.isAbsolute(moduleName)) {
const result = resolvePackage(context, moduleName, platform);
const result = resolveModulePath(context, moduleName, platform);
if (result.type === 'failed') {
throw new FailedToResolvePathError(result.candidates);
}
Expand Down Expand Up @@ -78,7 +78,7 @@ function resolve(
originModulePath.indexOf(path.sep, fromModuleParentIdx),
);
const absPath = path.join(originModuleDir, realModuleName);
const result = resolvePackage(context, absPath, platform);
const result = resolveModulePath(context, absPath, platform);
if (result.type === 'failed') {
throw new FailedToResolvePathError(result.candidates);
}
Expand Down Expand Up @@ -240,10 +240,11 @@ class MissingFileInHastePackageError extends Error {
}

/**
* In the NodeJS-style module resolution scheme we want to check potential
* paths both as directories and as files. For example, `/foo/bar` may resolve
* to `/foo/bar.js` (preferred), but it might also be `/foo/bar/index.js`, or
* even a package directory.
* Resolve a package entry point or subpath target.
*
* This should be used when resolving a bare import specifier prefixed with the
* package name. Use `resolveModulePath` instead to scope to legacy "browser"
* spec behaviour, which is also applicable to relative and absolute imports.
*/
function resolvePackage(
context: ResolutionContext,
Expand Down

0 comments on commit cd25c2b

Please sign in to comment.