From 36df5338367603cdda4d8683d323d433b47fb3d0 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Tue, 31 Mar 2026 12:01:45 +0100 Subject: [PATCH] Skip bare ext file resolution on ESM module resolution --- .../src/__tests__/esm-resolution-test.js | 335 ++++++++++++++++++ packages/metro-resolver/src/resolve.js | 27 +- 2 files changed, 356 insertions(+), 6 deletions(-) create mode 100644 packages/metro-resolver/src/__tests__/esm-resolution-test.js diff --git a/packages/metro-resolver/src/__tests__/esm-resolution-test.js b/packages/metro-resolver/src/__tests__/esm-resolution-test.js new file mode 100644 index 0000000000..f60c0420e0 --- /dev/null +++ b/packages/metro-resolver/src/__tests__/esm-resolution-test.js @@ -0,0 +1,335 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + * @oncall react_native + */ + +'use strict'; + +import type {ResolutionContext} from '../index'; + +import {createPackageAccessors, createResolutionContext} from './utils'; + +const Resolver = require('../index'); + +describe('ESM bare specifier resolution skips sibling file lookups in node_modules', () => { + const fileMap = { + '/root/src/main.js': '', + // A normal package with package.json and main entry + '/root/node_modules/invariant/package.json': JSON.stringify({ + name: 'invariant', + main: 'index.js', + }), + '/root/node_modules/invariant/index.js': '', + '/root/node_modules/invariant/lib/utils.js': '', + // A file sitting directly in node_modules (CJS pattern) + '/root/node_modules/invariant.web.js': '', + '/root/node_modules/invariant.js': '', + // Scoped package + '/root/node_modules/@scope/pkg/package.json': JSON.stringify({ + name: '@scope/pkg', + main: 'index.js', + }), + '/root/node_modules/@scope/pkg/index.js': '', + '/root/node_modules/@scope/pkg/utils.js': '', + // Platform-specific file as sibling to scoped package dir + '/root/node_modules/@scope/pkg.web.js': '', + }; + + const baseContext: ResolutionContext = { + ...createResolutionContext(fileMap), + originModulePath: '/root/src/main.js', + }; + + describe('bare package import (no subpath)', () => { + test('CJS resolves bare import to sibling file when it exists', () => { + // In CJS, resolveFile runs first, finding invariant.js as a sibling + expect( + Resolver.resolve( + {...baseContext, isESMImport: false}, + 'invariant', + null, + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/invariant.js', + }); + }); + + test('ESM resolves bare import via package.json main, not sibling file', () => { + // In ESM, resolveFile is skipped for bare package root imports, + // so it falls through to resolvePackageEntryPoint + expect( + Resolver.resolve( + {...baseContext, isESMImport: true}, + 'invariant', + null, + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/invariant/index.js', + }); + }); + + test('ESM does not resolve to a sibling file like node_modules/invariant.web.js', () => { + // Remove the package directory to force file fallback + const fileMapNoDir = { + '/root/src/main.js': '', + '/root/node_modules/invariant.web.js': '', + '/root/node_modules/invariant.js': '', + }; + + const context: ResolutionContext = { + ...createResolutionContext(fileMapNoDir), + originModulePath: '/root/src/main.js', + isESMImport: true, + }; + + // ESM should NOT resolve bare 'invariant' to node_modules/invariant.js + expect(() => Resolver.resolve(context, 'invariant', null)).toThrow(); + }); + + test('CJS can resolve to a standalone file in node_modules', () => { + const fileMapNoDir = { + '/root/src/main.js': '', + '/root/node_modules/invariant.web.js': '', + '/root/node_modules/invariant.js': '', + }; + + const context: ResolutionContext = { + ...createResolutionContext(fileMapNoDir), + originModulePath: '/root/src/main.js', + isESMImport: false, + }; + + // CJS should still resolve bare 'invariant' to node_modules/invariant.js + expect(Resolver.resolve(context, 'invariant', null)).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/invariant.js', + }); + }); + + test('ESM does not try platform extensions as sibling files for bare import', () => { + const fileSystemLookup = jest.fn(baseContext.fileSystemLookup); + const context: ResolutionContext = { + ...baseContext, + fileSystemLookup, + isESMImport: true, + }; + + Resolver.resolve(context, 'invariant', null); + + // Should NOT have looked up any of these sibling file paths + const lookedUpPaths = fileSystemLookup.mock.calls.map(c => c[0]); + expect(lookedUpPaths).not.toContain( + '/root/node_modules/invariant.web.js', + ); + expect(lookedUpPaths).not.toContain('/root/node_modules/invariant.js'); + expect(lookedUpPaths).not.toContain( + '/root/node_modules/invariant.native.js', + ); + // But should have looked up the directory + expect(lookedUpPaths).toContain('/root/node_modules'); + }); + + test('CJS tries file extensions as sibling files for bare import', () => { + const fileSystemLookup = jest.fn(baseContext.fileSystemLookup); + const context: ResolutionContext = { + ...baseContext, + fileSystemLookup, + isESMImport: false, + }; + + Resolver.resolve(context, 'invariant', null); + + // CJS should try sibling file paths (even though the package resolves + // first in this case, the file resolution runs in resolveModulePath) + const lookedUpPaths = fileSystemLookup.mock.calls.map(c => c[0]); + // The invariant directory exists as a package, so resolveFile is called + // within resolveModulePath - it looks for the bare file first + expect(lookedUpPaths).toContain('/root/node_modules/invariant'); + }); + }); + + describe('scoped bare package import', () => { + test('ESM resolves scoped package via package.json main', () => { + expect( + Resolver.resolve( + {...baseContext, isESMImport: true}, + '@scope/pkg', + null, + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/@scope/pkg/index.js', + }); + }); + + test('ESM does not try file extensions on scoped package name', () => { + const fileSystemLookup = jest.fn(baseContext.fileSystemLookup); + const context: ResolutionContext = { + ...baseContext, + fileSystemLookup, + isESMImport: true, + }; + + Resolver.resolve(context, '@scope/pkg', null); + + const lookedUpPaths = fileSystemLookup.mock.calls.map(c => c[0]); + expect(lookedUpPaths).not.toContain( + '/root/node_modules/@scope/pkg.web.js', + ); + expect(lookedUpPaths).not.toContain('/root/node_modules/@scope/pkg.js'); + }); + }); + + describe('subpath import (not package root)', () => { + test('ESM resolves subpath with file extensions', () => { + expect( + Resolver.resolve( + {...baseContext, isESMImport: true}, + 'invariant/lib/utils', + null, + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/invariant/lib/utils.js', + }); + }); + + test('CJS resolves subpath with file extensions', () => { + expect( + Resolver.resolve( + {...baseContext, isESMImport: false}, + 'invariant/lib/utils', + null, + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/invariant/lib/utils.js', + }); + }); + + test('ESM subpath import still tries platform extensions inside the package', () => { + const fileMapWithPlatform = { + ...fileMap, + '/root/node_modules/invariant/lib/utils.web.js': '', + }; + + const context: ResolutionContext = { + ...createResolutionContext(fileMapWithPlatform), + originModulePath: '/root/src/main.js', + isESMImport: true, + }; + + expect(Resolver.resolve(context, 'invariant/lib/utils', 'web')).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/invariant/lib/utils.web.js', + }); + }); + + test('ESM scoped subpath import resolves with file extensions', () => { + expect( + Resolver.resolve( + {...baseContext, isESMImport: true}, + '@scope/pkg/utils', + null, + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/@scope/pkg/utils.js', + }); + }); + }); + + describe('with package exports enabled', () => { + const exportsFileMap = { + '/root/src/main.js': '', + '/root/node_modules/pkg-with-exports/package.json': JSON.stringify({ + name: 'pkg-with-exports', + main: 'lib/index.js', + exports: { + '.': './lib/index.js', + './utils': './lib/utils.js', + }, + }), + '/root/node_modules/pkg-with-exports/lib/index.js': '', + '/root/node_modules/pkg-with-exports/lib/utils.js': '', + // A sibling file that should never be picked + '/root/node_modules/pkg-with-exports.js': '', + }; + + const exportsContext: ResolutionContext = { + ...createResolutionContext(exportsFileMap), + ...createPackageAccessors(exportsFileMap), + originModulePath: '/root/src/main.js', + unstable_enablePackageExports: true, + }; + + test('ESM resolves via exports field, not sibling file', () => { + expect( + Resolver.resolve( + {...exportsContext, isESMImport: true}, + 'pkg-with-exports', + null, + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/pkg-with-exports/lib/index.js', + }); + }); + + test('ESM subpath resolves via exports field', () => { + expect( + Resolver.resolve( + {...exportsContext, isESMImport: true}, + 'pkg-with-exports/utils', + null, + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/pkg-with-exports/lib/utils.js', + }); + }); + }); + + describe('relative and absolute imports are unaffected', () => { + const relFileMap = { + '/root/src/main.js': '', + '/root/src/utils.js': '', + '/root/src/utils.web.js': '', + }; + + const relContext: ResolutionContext = { + ...createResolutionContext(relFileMap), + originModulePath: '/root/src/main.js', + }; + + test('ESM relative import still tries platform extensions', () => { + expect( + Resolver.resolve({...relContext, isESMImport: true}, './utils', 'web'), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/src/utils.web.js', + }); + }); + + test('ESM absolute import still tries platform extensions', () => { + expect( + Resolver.resolve( + {...relContext, isESMImport: true}, + '/root/src/utils', + 'web', + ), + ).toEqual({ + type: 'sourceFile', + filePath: '/root/src/utils.web.js', + }); + }); + }); +}); diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index ebde8adf27..3c7100e2db 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -60,7 +60,7 @@ export default function resolve( } if (isRelativeImport(specifier) || path.isAbsolute(specifier)) { - const result = resolveModulePath(context, specifier, platform); + const result = resolveModulePath(context, specifier, platform, false); if (result.type === 'failed') { throw new FailedToResolvePathError(result.candidates); } @@ -132,7 +132,7 @@ export default function resolve( originModulePath.indexOf(path.sep, fromModuleParentIdx), ); const absPath = path.join(originModuleDir, redirectedSpecifier); - const result = resolveModulePath(context, absPath, platform); + const result = resolveModulePath(context, absPath, platform, false); if (result.type === 'failed') { throw new FailedToResolvePathError(result.candidates); } @@ -232,10 +232,12 @@ export default function resolve( newPackageName, parsedSpecifier.posixSubpath, ); + const isPackageRoot = parsedSpecifier.posixSubpath === '.'; const resolution = resolveModuleFromTargetPath( context, platform, extraNodeModulePath, + isPackageRoot, ); if (resolution != null) { return resolution; @@ -269,12 +271,14 @@ function resolveFromNodeModulesPath( if (!lookupResult.exists || lookupResult.type !== 'd') { return null; } + const isPackageRoot = parsedSpecifier.posixSubpath === '.'; return resolveModuleFromTargetPath( context, platform, nodeModulesPath + path.sep + posixToSystemPath(parsedSpecifier.normalizedSpecifier), + isPackageRoot, ); } @@ -282,6 +286,7 @@ function resolveModuleFromTargetPath( context: ResolutionContext, platform: string | null, targetPath: string, + isPackageRoot: boolean, ): Resolution | null { const candidate = redirectModulePath(context, targetPath); if (candidate === false) { @@ -290,7 +295,7 @@ function resolveModuleFromTargetPath( // candidate should be absolute here - we assume that redirectModulePath // always returns an absolute path when given an absolute path. - const result = resolvePackage(context, candidate, platform); + const result = resolvePackage(context, candidate, platform, isPackageRoot); if (result.type === 'resolved') { return result.resolution; } @@ -382,6 +387,7 @@ function resolveModulePath( context: ResolutionContext, toModuleName: string, platform: string | null, + skipFileResolution: boolean, ): Result { // System-separated absolute path const modulePath = path.isAbsolute(toModuleName) @@ -400,7 +406,9 @@ function resolveModulePath( const fileResult: ?Result = // require('./foo/') should never resolve to ./foo.js - a trailing slash // implies we should resolve as a directory only. - redirectedPath.endsWith(path.sep) + // For ESM bare package imports (isPackageRoot), skip file resolution - + // e.g. don't look for node_modules/invariant.web.js as a sibling file. + redirectedPath.endsWith(path.sep) || skipFileResolution ? null : resolveFile(context, dirPath, fileName, platform); @@ -434,7 +442,7 @@ function resolveHastePackage( return failedFor(); } const potentialModulePath = path.join(packageJsonPath, '..', pathInModule); - const result = resolvePackage(context, potentialModulePath, platform); + const result = resolvePackage(context, potentialModulePath, platform, false); if (result.type === 'resolved') { return result; } @@ -485,6 +493,7 @@ function resolvePackage( */ absoluteCandidatePath: string, platform: string | null, + isPackageRoot: boolean, ): Result { if (context.unstable_enablePackageExports) { const pkg = context.getPackageForModule(absoluteCandidatePath); @@ -522,7 +531,13 @@ function resolvePackage( } } - return resolveModulePath(context, absoluteCandidatePath, platform); + const skipFileResolution = !!context.isESMImport && isPackageRoot; + return resolveModulePath( + context, + absoluteCandidatePath, + platform, + skipFileResolution, + ); } /**