Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore special case workaround for @babel/runtime for compatibility with RN defaults #1468

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/metro-resolver/src/PackageExportsResolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export function resolvePackageTargetFromExports(

const filePath = path.join(
packagePath,
patternMatch != null ? target.replace('*', patternMatch) : target,
patternMatch != null ? target.replaceAll('*', patternMatch) : target,
);

if (isAssetFile(filePath, context.assetExts)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/metro-resolver/src/PackageImportsResolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function resolvePackageTargetFromImports(

const filePath = path.join(
packagePath,
patternMatch != null ? target.replace('*', patternMatch) : target,
patternMatch != null ? target.replaceAll('*', patternMatch) : target,
);

if (isAssetFile(filePath, context.assetExts)) {
Expand Down
78 changes: 78 additions & 0 deletions packages/metro-resolver/src/__tests__/package-exports-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -510,17 +510,32 @@ describe('with package exports resolution enabled', () => {
'./features/bar/*.js': {
'react-native': null,
},
'./node/*': './lib/node/*.js',
'./*': './misc/*.js',
'./node/*/types': './types/*/types.d.ts',
'./assets/*': './assets/*',
'./repeated*/repeated': './repeated*/repeated.js',
'./*/private': './src/forbidden.js',
'./data.*': './data/*/data.*',
},
}),
'/root/node_modules/test-pkg/lib/node/test.js': '',
'/root/node_modules/test-pkg/lib/node/foo/public.js': '',
'/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': '',
'/root/node_modules/test-pkg/src/features/baz.native.js': '',
'/root/node_modules/test-pkg/src/features/node_modules/foo/index.js':
'',
'/root/node_modules/test-pkg/src/forbidden.js': '',
'/root/node_modules/test-pkg/misc/repeated.js': '',
'/root/node_modules/test-pkg/repeated/repeated.js': '',
'/root/node_modules/test-pkg/repeated/repeated/repeated.js': '',
'/root/node_modules/test-pkg/types/foo/types.d.ts': '',
'/root/node_modules/test-pkg/assets/Logo.js': '',
'/root/node_modules/test-pkg/data/json/data.json': '',
'/root/node_modules/test-pkg/data/csv/data.csv': '',
}),
originModulePath: '/root/src/main.js',
unstable_enablePackageExports: true,
Expand Down Expand Up @@ -577,6 +592,69 @@ describe('with package exports resolution enabled', () => {
`);
});

test('should resolve prefixed wildcard export, using the most specific export path', () => {
const context = baseContext;

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

test('longer patterns have higher specificity if bases are equal', () => {
const context = baseContext;
expect(
Resolver.resolve(context, 'test-pkg/node/foo/public', null),
).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/lib/node/foo/public.js',
});

expect(
Resolver.resolve(context, 'test-pkg/node/foo/types', null),
).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/types/foo/types.d.ts',
});
});

test('patternBase and patternTrailer must match non-overlapping ends of matchKey', () => {
// ./repeated/repeated *should* match ./repeated*/repeated
expect(
Resolver.resolve(baseContext, 'test-pkg/repeated/repeated', null),
).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/repeated/repeated.js',
});

// ./repeated should *not* match ./repeated*/repeated
expect(Resolver.resolve(baseContext, 'test-pkg/repeated', null)).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/misc/repeated.js',
});
});

test('patterns can resolve to static targets', () => {
const context = baseContext;
expect(Resolver.resolve(context, 'test-pkg/foo/private', null)).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/src/forbidden.js',
});
});

test('patterns can resolve to targets with multiple *', () => {
const context = baseContext;
expect(Resolver.resolve(context, 'test-pkg/data.json', null)).toEqual({
type: 'sourceFile',
filePath: '/root/node_modules/test-pkg/data/json/data.json',
});

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

describe('package encapsulation', () => {
test('[nonstrict] should fall back to "browser" spec resolution and log inaccessible import warning', () => {
const logWarning = jest.fn();
Expand Down
23 changes: 22 additions & 1 deletion packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,30 @@ function resolvePackage(
const exportsField = pkg?.packageJson.exports;

if (pkg != null && exportsField != null) {
// HACK!: Do not assert the "import" condition for `@babel/runtime`. This
// is a workaround for ESM <-> CJS interop, as we need the CJS versions of
// `@babel/runtime` helpers.
//
// This is not necessary when Metro is correctly configured, but we're
// restoring this hack that pre-dates isESMImport to accommodate the
// defaults in @react-native/metro-config <0.78.2 / <0.77.2, and possibly
// Expo SDK 52: https://github.com/facebook/metro/issues/1464.
let contextWithOverrides = context;
if (
pkg.packageJson.name === '@babel/runtime' &&
context.isESMImport !== true
) {
contextWithOverrides = {
...context,
unstable_conditionNames: context.unstable_conditionNames.filter(
condition => condition !== 'import',
),
};
}

try {
const packageExportsResult = resolvePackageTargetFromExports(
context,
contextWithOverrides,
pkg.rootPath,
absoluteCandidatePath,
pkg.packageRelativePath,
Expand Down
22 changes: 13 additions & 9 deletions packages/metro-resolver/src/utils/matchSubpathFromExportsLike.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,23 @@ export function matchSubpathFromExportsLike(
// Attempt to match after expanding any subpath pattern keys
if (target == null) {
// Gather keys which are subpath patterns in descending order of specificity
// For ordering, see `PATTERN_KEY_COMPARE` in:
// https://nodejs.org/api/esm.html#resolution-algorithm-specification
const expansionKeys = [...exportsLikeMapAfterConditions.keys()]
.filter(key => key.includes('*'))
.sort(key => key.split('*')[0].length)
.reverse();
.map(key => ({key, baseLength: key.indexOf('*')}))
.filter(data => data.baseLength !== -1)
.sort((a, b) => {
if (a.baseLength === b.baseLength) {
// If wildcards are in equal positions, the longer key is more
// specific
return b.key.length - a.key.length;
}
return b.baseLength - a.baseLength;
});

for (const key of expansionKeys) {
for (const {key} of expansionKeys) {
const value = exportsLikeMapAfterConditions.get(key);

// Skip invalid values (must include a single '*' or be `null`)
if (typeof value === 'string' && value.split('*').length !== 2) {
break;
}

patternMatch = matchSubpathPattern(key, subpath);

if (patternMatch != null) {
Expand Down
7 changes: 6 additions & 1 deletion packages/metro-resolver/src/utils/matchSubpathPattern.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ export function matchSubpathPattern(
): string | null {
const [patternBase, patternTrailer] = subpathPattern.split('*');

if (subpath.startsWith(patternBase) && subpath.endsWith(patternTrailer)) {
if (
subpath.startsWith(patternBase) &&
(patternTrailer.length === 0 ||
(subpath.endsWith(patternTrailer) &&
subpath.length >= subpathPattern.length))
) {
return subpath.substring(
patternBase.length,
subpath.length - patternTrailer.length,
Expand Down
Loading