Skip to content

Commit 4e8f2e6

Browse files
kittenkitten
committed
Skip bare ext file resolution on ESM module resolution
1 parent bfeba10 commit 4e8f2e6

2 files changed

Lines changed: 347 additions & 3 deletions

File tree

‎packages/metro-resolver/src/__tests__/esm-resolution-test.js‎

Lines changed: 335 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,335 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
* @oncall react_native
10+
*/
11+
12+
'use strict';
13+
14+
import type {ResolutionContext} from '../index';
15+
16+
import {createPackageAccessors, createResolutionContext} from './utils';
17+
18+
const Resolver = require('../index');
19+
20+
describe('ESM bare specifier resolution skips sibling file lookups in node_modules', () => {
21+
const fileMap = {
22+
'/root/src/main.js': '',
23+
// A normal package with package.json and main entry
24+
'/root/node_modules/invariant/package.json': JSON.stringify({
25+
name: 'invariant',
26+
main: 'index.js',
27+
}),
28+
'/root/node_modules/invariant/index.js': '',
29+
'/root/node_modules/invariant/lib/utils.js': '',
30+
// A file sitting directly in node_modules (CJS pattern)
31+
'/root/node_modules/invariant.web.js': '',
32+
'/root/node_modules/invariant.js': '',
33+
// Scoped package
34+
'/root/node_modules/@scope/pkg/package.json': JSON.stringify({
35+
name: '@scope/pkg',
36+
main: 'index.js',
37+
}),
38+
'/root/node_modules/@scope/pkg/index.js': '',
39+
'/root/node_modules/@scope/pkg/utils.js': '',
40+
// Platform-specific file as sibling to scoped package dir
41+
'/root/node_modules/@scope/pkg.web.js': '',
42+
};
43+
44+
const baseContext: ResolutionContext = {
45+
...createResolutionContext(fileMap),
46+
originModulePath: '/root/src/main.js',
47+
};
48+
49+
describe('bare package import (no subpath)', () => {
50+
test('CJS resolves bare import to sibling file when it exists', () => {
51+
// In CJS, resolveFile runs first, finding invariant.js as a sibling
52+
expect(
53+
Resolver.resolve(
54+
{...baseContext, isESMImport: false},
55+
'invariant',
56+
null,
57+
),
58+
).toEqual({
59+
type: 'sourceFile',
60+
filePath: '/root/node_modules/invariant.js',
61+
});
62+
});
63+
64+
test('ESM resolves bare import via package.json main, not sibling file', () => {
65+
// In ESM, resolveFile is skipped for bare package root imports,
66+
// so it falls through to resolvePackageEntryPoint
67+
expect(
68+
Resolver.resolve(
69+
{...baseContext, isESMImport: true},
70+
'invariant',
71+
null,
72+
),
73+
).toEqual({
74+
type: 'sourceFile',
75+
filePath: '/root/node_modules/invariant/index.js',
76+
});
77+
});
78+
79+
test('ESM does not resolve to a sibling file like node_modules/invariant.web.js', () => {
80+
// Remove the package directory to force file fallback
81+
const fileMapNoDir = {
82+
'/root/src/main.js': '',
83+
'/root/node_modules/invariant.web.js': '',
84+
'/root/node_modules/invariant.js': '',
85+
};
86+
87+
const context: ResolutionContext = {
88+
...createResolutionContext(fileMapNoDir),
89+
originModulePath: '/root/src/main.js',
90+
isESMImport: true,
91+
};
92+
93+
// ESM should NOT resolve bare 'invariant' to node_modules/invariant.js
94+
expect(() => Resolver.resolve(context, 'invariant', null)).toThrow();
95+
});
96+
97+
test('CJS can resolve to a standalone file in node_modules', () => {
98+
const fileMapNoDir = {
99+
'/root/src/main.js': '',
100+
'/root/node_modules/invariant.web.js': '',
101+
'/root/node_modules/invariant.js': '',
102+
};
103+
104+
const context: ResolutionContext = {
105+
...createResolutionContext(fileMapNoDir),
106+
originModulePath: '/root/src/main.js',
107+
isESMImport: false,
108+
};
109+
110+
// CJS should still resolve bare 'invariant' to node_modules/invariant.js
111+
expect(Resolver.resolve(context, 'invariant', null)).toEqual({
112+
type: 'sourceFile',
113+
filePath: '/root/node_modules/invariant.js',
114+
});
115+
});
116+
117+
test('ESM does not try platform extensions as sibling files for bare import', () => {
118+
const fileSystemLookup = jest.fn(baseContext.fileSystemLookup);
119+
const context: ResolutionContext = {
120+
...baseContext,
121+
fileSystemLookup,
122+
isESMImport: true,
123+
};
124+
125+
Resolver.resolve(context, 'invariant', null);
126+
127+
// Should NOT have looked up any of these sibling file paths
128+
const lookedUpPaths = fileSystemLookup.mock.calls.map(c => c[0]);
129+
expect(lookedUpPaths).not.toContain(
130+
'/root/node_modules/invariant.web.js',
131+
);
132+
expect(lookedUpPaths).not.toContain('/root/node_modules/invariant.js');
133+
expect(lookedUpPaths).not.toContain(
134+
'/root/node_modules/invariant.native.js',
135+
);
136+
// But should have looked up the directory
137+
expect(lookedUpPaths).toContain('/root/node_modules');
138+
});
139+
140+
test('CJS tries file extensions as sibling files for bare import', () => {
141+
const fileSystemLookup = jest.fn(baseContext.fileSystemLookup);
142+
const context: ResolutionContext = {
143+
...baseContext,
144+
fileSystemLookup,
145+
isESMImport: false,
146+
};
147+
148+
Resolver.resolve(context, 'invariant', null);
149+
150+
// CJS should try sibling file paths (even though the package resolves
151+
// first in this case, the file resolution runs in resolveModulePath)
152+
const lookedUpPaths = fileSystemLookup.mock.calls.map(c => c[0]);
153+
// The invariant directory exists as a package, so resolveFile is called
154+
// within resolveModulePath - it looks for the bare file first
155+
expect(lookedUpPaths).toContain('/root/node_modules/invariant');
156+
});
157+
});
158+
159+
describe('scoped bare package import', () => {
160+
test('ESM resolves scoped package via package.json main', () => {
161+
expect(
162+
Resolver.resolve(
163+
{...baseContext, isESMImport: true},
164+
'@scope/pkg',
165+
null,
166+
),
167+
).toEqual({
168+
type: 'sourceFile',
169+
filePath: '/root/node_modules/@scope/pkg/index.js',
170+
});
171+
});
172+
173+
test('ESM does not try file extensions on scoped package name', () => {
174+
const fileSystemLookup = jest.fn(baseContext.fileSystemLookup);
175+
const context: ResolutionContext = {
176+
...baseContext,
177+
fileSystemLookup,
178+
isESMImport: true,
179+
};
180+
181+
Resolver.resolve(context, '@scope/pkg', null);
182+
183+
const lookedUpPaths = fileSystemLookup.mock.calls.map(c => c[0]);
184+
expect(lookedUpPaths).not.toContain(
185+
'/root/node_modules/@scope/pkg.web.js',
186+
);
187+
expect(lookedUpPaths).not.toContain('/root/node_modules/@scope/pkg.js');
188+
});
189+
});
190+
191+
describe('subpath import (not package root)', () => {
192+
test('ESM resolves subpath with file extensions', () => {
193+
expect(
194+
Resolver.resolve(
195+
{...baseContext, isESMImport: true},
196+
'invariant/lib/utils',
197+
null,
198+
),
199+
).toEqual({
200+
type: 'sourceFile',
201+
filePath: '/root/node_modules/invariant/lib/utils.js',
202+
});
203+
});
204+
205+
test('CJS resolves subpath with file extensions', () => {
206+
expect(
207+
Resolver.resolve(
208+
{...baseContext, isESMImport: false},
209+
'invariant/lib/utils',
210+
null,
211+
),
212+
).toEqual({
213+
type: 'sourceFile',
214+
filePath: '/root/node_modules/invariant/lib/utils.js',
215+
});
216+
});
217+
218+
test('ESM subpath import still tries platform extensions inside the package', () => {
219+
const fileMapWithPlatform = {
220+
...fileMap,
221+
'/root/node_modules/invariant/lib/utils.web.js': '',
222+
};
223+
224+
const context: ResolutionContext = {
225+
...createResolutionContext(fileMapWithPlatform),
226+
originModulePath: '/root/src/main.js',
227+
isESMImport: true,
228+
};
229+
230+
expect(Resolver.resolve(context, 'invariant/lib/utils', 'web')).toEqual({
231+
type: 'sourceFile',
232+
filePath: '/root/node_modules/invariant/lib/utils.web.js',
233+
});
234+
});
235+
236+
test('ESM scoped subpath import resolves with file extensions', () => {
237+
expect(
238+
Resolver.resolve(
239+
{...baseContext, isESMImport: true},
240+
'@scope/pkg/utils',
241+
null,
242+
),
243+
).toEqual({
244+
type: 'sourceFile',
245+
filePath: '/root/node_modules/@scope/pkg/utils.js',
246+
});
247+
});
248+
});
249+
250+
describe('with package exports enabled', () => {
251+
const exportsFileMap = {
252+
'/root/src/main.js': '',
253+
'/root/node_modules/pkg-with-exports/package.json': JSON.stringify({
254+
name: 'pkg-with-exports',
255+
main: 'lib/index.js',
256+
exports: {
257+
'.': './lib/index.js',
258+
'./utils': './lib/utils.js',
259+
},
260+
}),
261+
'/root/node_modules/pkg-with-exports/lib/index.js': '',
262+
'/root/node_modules/pkg-with-exports/lib/utils.js': '',
263+
// A sibling file that should never be picked
264+
'/root/node_modules/pkg-with-exports.js': '',
265+
};
266+
267+
const exportsContext: ResolutionContext = {
268+
...createResolutionContext(exportsFileMap),
269+
...createPackageAccessors(exportsFileMap),
270+
originModulePath: '/root/src/main.js',
271+
unstable_enablePackageExports: true,
272+
};
273+
274+
test('ESM resolves via exports field, not sibling file', () => {
275+
expect(
276+
Resolver.resolve(
277+
{...exportsContext, isESMImport: true},
278+
'pkg-with-exports',
279+
null,
280+
),
281+
).toEqual({
282+
type: 'sourceFile',
283+
filePath: '/root/node_modules/pkg-with-exports/lib/index.js',
284+
});
285+
});
286+
287+
test('ESM subpath resolves via exports field', () => {
288+
expect(
289+
Resolver.resolve(
290+
{...exportsContext, isESMImport: true},
291+
'pkg-with-exports/utils',
292+
null,
293+
),
294+
).toEqual({
295+
type: 'sourceFile',
296+
filePath: '/root/node_modules/pkg-with-exports/lib/utils.js',
297+
});
298+
});
299+
});
300+
301+
describe('relative and absolute imports are unaffected', () => {
302+
const relFileMap = {
303+
'/root/src/main.js': '',
304+
'/root/src/utils.js': '',
305+
'/root/src/utils.web.js': '',
306+
};
307+
308+
const relContext: ResolutionContext = {
309+
...createResolutionContext(relFileMap),
310+
originModulePath: '/root/src/main.js',
311+
};
312+
313+
test('ESM relative import still tries platform extensions', () => {
314+
expect(
315+
Resolver.resolve({...relContext, isESMImport: true}, './utils', 'web'),
316+
).toEqual({
317+
type: 'sourceFile',
318+
filePath: '/root/src/utils.web.js',
319+
});
320+
});
321+
322+
test('ESM absolute import still tries platform extensions', () => {
323+
expect(
324+
Resolver.resolve(
325+
{...relContext, isESMImport: true},
326+
'/root/src/utils',
327+
'web',
328+
),
329+
).toEqual({
330+
type: 'sourceFile',
331+
filePath: '/root/src/utils.web.js',
332+
});
333+
});
334+
});
335+
});

‎packages/metro-resolver/src/resolve.js‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,9 @@ export default function resolve(
223223

224224
// candidate should be absolute here - we assume that redirectModulePath
225225
// always returns an absolute path when given an absolute path.
226-
const result = resolvePackage(context, candidate, platform);
226+
const result = resolvePackage(context, candidate, platform, {
227+
isPackageRoot: parsedSpecifier.posixSubpath === '.',
228+
});
227229
if (result.type === 'resolved') {
228230
return result.resolution;
229231
}
@@ -293,6 +295,7 @@ function resolveModulePath(
293295
context: ResolutionContext,
294296
toModuleName: string,
295297
platform: string | null,
298+
options?: $ReadOnly<{skipFileResolution?: boolean}>,
296299
): Result<Resolution, FileAndDirCandidates> {
297300
// System-separated absolute path
298301
const modulePath = path.isAbsolute(toModuleName)
@@ -311,7 +314,9 @@ function resolveModulePath(
311314
const fileResult: ?Result<Resolution, FileCandidates> =
312315
// require('./foo/') should never resolve to ./foo.js - a trailing slash
313316
// implies we should resolve as a directory only.
314-
redirectedPath.endsWith(path.sep)
317+
// For ESM bare package imports (isPackageRoot), skip file resolution -
318+
// e.g. don't look for node_modules/invariant.web.js as a sibling file.
319+
redirectedPath.endsWith(path.sep) || options?.skipFileResolution === true
315320
? null
316321
: resolveFile(context, dirPath, fileName, platform);
317322

@@ -396,6 +401,7 @@ function resolvePackage(
396401
*/
397402
absoluteCandidatePath: string,
398403
platform: string | null,
404+
options?: $ReadOnly<{isPackageRoot?: boolean}>,
399405
): Result<Resolution, FileAndDirCandidates> {
400406
if (context.unstable_enablePackageExports) {
401407
const pkg = context.getPackageForModule(absoluteCandidatePath);
@@ -433,7 +439,10 @@ function resolvePackage(
433439
}
434440
}
435441

436-
return resolveModulePath(context, absoluteCandidatePath, platform);
442+
return resolveModulePath(context, absoluteCandidatePath, platform, {
443+
skipFileResolution:
444+
context.isESMImport === true && options?.isPackageRoot === true,
445+
});
437446
}
438447

439448
/**

0 commit comments

Comments
 (0)