diff --git a/packages/metro-resolver/src/__tests__/index-test.js b/packages/metro-resolver/src/__tests__/index-test.js index 4bec19800e..ed835d32a1 100644 --- a/packages/metro-resolver/src/__tests__/index-test.js +++ b/packages/metro-resolver/src/__tests__/index-test.js @@ -32,6 +32,7 @@ const fileMap = { name: 'invalid', main: 'main', }), + '/root/node_modules/flat-file-in-node-modules.js': '', '/node_modules/root-module/main.js': '', '/node_modules/root-module/package.json': JSON.stringify({ name: 'root-module', @@ -110,8 +111,8 @@ it('does not resolve a relative path ending in a slash as a file', () => { file: null, dir: { type: 'sourceFile', - filePathPrefix: '/root/project/bar/index', - candidateExts: ['', '.js', '.jsx', '.json', '.ts', '.tsx'], + filePathPrefix: '/root/project/bar/', + candidateExts: [], }, }), ); @@ -124,6 +125,13 @@ it('resolves a package in `node_modules`', () => { }); }); +it('resolves a standalone file in `node_modules`', () => { + expect(Resolver.resolve(CONTEXT, 'flat-file-in-node-modules', null)).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/flat-file-in-node-modules.js', + }); +}); + it('fails to resolve a relative path', () => { try { Resolver.resolve(CONTEXT, './apple', null); @@ -134,8 +142,8 @@ it('fails to resolve a relative path', () => { } expect(error.candidates).toEqual({ dir: { - candidateExts: ['', '.js', '.jsx', '.json', '.ts', '.tsx'], - filePathPrefix: '/root/project/apple/index', + candidateExts: [], + filePathPrefix: '/root/project/apple', type: 'sourceFile', }, file: { @@ -326,7 +334,7 @@ it('throws a descriptive error when a file inside a Haste package cannot be reso "While resolving module \`some-package/subdir/does-not-exist\`, the Haste package \`some-package\` was found. However the module \`subdir/does-not-exist\` could not be found within the package. Indeed, none of these files exist: * \`/haste/some-package/subdir/does-not-exist(.js|.jsx|.json|.ts|.tsx)\` - * \`/haste/some-package/subdir/does-not-exist/index(.js|.jsx|.json|.ts|.tsx)\`" + * \`/haste/some-package/subdir/does-not-exist\`" `); }); diff --git a/packages/metro-resolver/src/__tests__/utils.js b/packages/metro-resolver/src/__tests__/utils.js index c66f81eee6..76f8562755 100644 --- a/packages/metro-resolver/src/__tests__/utils.js +++ b/packages/metro-resolver/src/__tests__/utils.js @@ -30,6 +30,17 @@ type MockFileMap = $ReadOnly<{ export function createResolutionContext( fileMap: MockFileMap = {}, ): $Diff { + const directorySet = new Set(); + for (const filePath of Object.keys(fileMap)) { + let currentDir = filePath; + let prevDir; + do { + prevDir = currentDir; + currentDir = path.dirname(currentDir); + directorySet.add(currentDir); + } while (currentDir !== prevDir); + } + return { dev: true, allowHaste: true, @@ -56,12 +67,20 @@ export function createResolutionContext( web: ['browser'], }, unstable_enablePackageExports: false, - unstable_fileSystemLookup: filePath => { + unstable_fileSystemLookup: inputPath => { + // Normalise and remove any trailing slash. + const filePath = path.resolve(inputPath); const candidate = fileMap[filePath]; if (typeof candidate === 'string') { return {exists: true, type: 'f', realPath: filePath}; } - if (candidate == null || candidate.realPath == null) { + if (candidate == null) { + if (directorySet.has(filePath)) { + return {exists: true, type: 'd', realPath: filePath}; + } + return {exists: false}; + } + if (candidate.realPath == null) { return {exists: false}; } return { diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index 900ba7ef87..3aa4c4f899 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -85,7 +85,12 @@ function resolve( return result.resolution; } - if (context.allowHaste && !isDirectImport) { + /** + * At this point, realModuleName is not a "direct" (absolute or relative) + * import, so it's either Haste name or a package specifier. + */ + + if (context.allowHaste) { const normalizedName = normalizePath(realModuleName); const result = resolveHasteName(context, normalizedName, platform); if (result.type === 'resolved') { @@ -93,6 +98,10 @@ function resolve( } } + /** + * realModuleName is now a package specifier. + */ + const {disableHierarchicalLookup} = context; const nodeModulesPaths = []; @@ -102,7 +111,10 @@ function resolve( let candidate; do { candidate = next; - nodeModulesPaths.push(path.join(candidate, 'node_modules')); + const nodeModulesPath = candidate.endsWith(path.sep) + ? candidate + 'node_modules' + : candidate + path.sep + 'node_modules'; + nodeModulesPaths.push(nodeModulesPath); next = path.dirname(candidate); } while (candidate !== next); } @@ -111,25 +123,38 @@ function resolve( nodeModulesPaths.push(...context.nodeModulesPaths); const extraPaths = []; + + const parsedSpecifier = parsePackageSpecifier(realModuleName); + const {extraNodeModules} = context; - if (extraNodeModules) { - let bits = path.normalize(moduleName).split(path.sep); - let packageName; - // Normalize packageName and bits for scoped modules - if (bits.length >= 2 && bits[0].startsWith('@')) { - packageName = bits.slice(0, 2).join('/'); - bits = bits.slice(1); - } else { - packageName = bits[0]; - } - if (extraNodeModules[packageName]) { - bits[0] = extraNodeModules[packageName]; - extraPaths.push(path.join.apply(path, bits)); - } + if (extraNodeModules && extraNodeModules[parsedSpecifier.packageName]) { + const newPackageName = extraNodeModules[parsedSpecifier.packageName]; + extraPaths.push(path.join(newPackageName, parsedSpecifier.posixSubpath)); } const allDirPaths = nodeModulesPaths - .map(nodeModulePath => path.join(nodeModulePath, realModuleName)) + .map(nodeModulePath => { + let lookupResult = null; + if (context.unstable_fileSystemLookup) { + // Insight: The module can only exist if there is a `node_modules` at + // this path. Redirections cannot succeed, because we will never look + // beyond a node_modules path segment for finding the closest + // package.json. Moreover, if the specifier contains a '/' separator, + // the first part *must* be a real directory, because it is the + // shallowest path that can possibly contain a redirecting package.json. + const mustBeDirectory = + parsedSpecifier.posixSubpath !== '.' || + parsedSpecifier.packageName.length > parsedSpecifier.firstPart.length + ? nodeModulePath + path.sep + parsedSpecifier.firstPart + : nodeModulePath; + lookupResult = context.unstable_fileSystemLookup(mustBeDirectory); + if (!lookupResult.exists || lookupResult.type !== 'd') { + return null; + } + } + return path.join(nodeModulePath, realModuleName); + }) + .filter(Boolean) .concat(extraPaths); for (let i = 0; i < allDirPaths.length; ++i) { const candidate = context.redirectModulePath(allDirPaths[i]); @@ -149,6 +174,40 @@ function resolve( throw new FailedToResolveNameError(nodeModulesPaths, extraPaths); } +function parsePackageSpecifier(specifier: string) { + const normalized = + path.sep === '/' ? specifier : specifier.replaceAll('\\', '/'); + const firstSepIdx = normalized.indexOf('/'); + if (normalized.startsWith('@') && firstSepIdx !== -1) { + const secondSepIdx = normalized.indexOf('/', firstSepIdx + 1); + if (secondSepIdx === -1) { + return { + firstPart: normalized.slice(0, firstSepIdx), + packageName: normalized, + posixSubpath: '.', + }; + } + return { + firstPart: normalized.slice(0, firstSepIdx), + packageName: normalized.slice(0, secondSepIdx), + posixSubpath: '.' + normalized.slice(secondSepIdx), + }; + } + if (firstSepIdx === -1) { + return { + firstPart: normalized, + packageName: normalized, + posixSubpath: '.', + }; + } + const packageName = normalized.slice(0, firstSepIdx); + return { + firstPart: packageName, + packageName, + posixSubpath: '.' + normalized.slice(firstSepIdx), + }; +} + /** * Resolve any kind of module path, whether it's a file or a directory. * For example we may want to resolve './foobar'. The closest @@ -336,6 +395,19 @@ function resolvePackageEntryPoint( packagePath: string, platform: string | null, ): Result { + const dirLookup = context.unstable_fileSystemLookup?.(packagePath); + + if ( + dirLookup != null && + (dirLookup.exists === false || dirLookup.type !== 'd') + ) { + return failedFor({ + type: 'sourceFile', + filePathPrefix: packagePath, + candidateExts: [], + }); + } + const packageJsonPath = path.join(packagePath, 'package.json'); if (!context.doesFileExist(packageJsonPath)) { diff --git a/packages/metro/src/DeltaBundler/__tests__/__snapshots__/resolver-test.js.snap b/packages/metro/src/DeltaBundler/__tests__/__snapshots__/resolver-test.js.snap index 8b12b1c4c1..8bdc2b00e1 100644 --- a/packages/metro/src/DeltaBundler/__tests__/__snapshots__/resolver-test.js.snap +++ b/packages/metro/src/DeltaBundler/__tests__/__snapshots__/resolver-test.js.snap @@ -5,7 +5,7 @@ exports[`linux assets checks asset extensions case insensitively 1`] = ` None of these files exist: * asset.PNG(.native.js|.js|.native.json|.json) - * asset.PNG/index(.native.js|.js|.native.json|.json) + * asset.PNG 1 | import foo from 'bar'; > 2 | import a from './asset.PNG'; | ^ @@ -17,7 +17,7 @@ exports[`linux assets resolves asset files with resolution suffixes (matching ex None of these files exist: * c@2x.png - * c@2x.png/index(.native.js|.js|.native.json|.json) + * c@2x.png 1 | import foo from 'bar'; > 2 | import a from './c@2x.png'; | ^ @@ -29,7 +29,7 @@ exports[`linux assets resolves asset files with resolution suffixes (matching si None of these files exist: * a@1.5x.png - * a@1.5x.png/index(.native.js|.js|.native.json|.json) + * a@1.5x.png 1 | import foo from 'bar'; > 2 | import a from './a@1.5x.png'; | ^ @@ -41,7 +41,7 @@ exports[`linux assets resolves custom asset extensions when overriding assetExts None of these files exist: * asset2.png(.native.js|.js|.native.json|.json) - * asset2.png/index(.native.js|.js|.native.json|.json) + * asset2.png 1 | import foo from 'bar'; > 2 | import a from './asset2.png'; | ^ @@ -116,7 +116,7 @@ exports[`linux packages in node_modules/ browser field in package.json resolves None of these files exist: * node_modules/aPackage/foo.js(.native.js|.js|.native.json|.json) - * node_modules/aPackage/foo.js/index(.native.js|.js|.native.json|.json) + * node_modules/aPackage/foo.js 1 | import foo from 'bar'; > 2 | import f from './foo.js'; | ^ @@ -146,7 +146,7 @@ exports[`linux packages in node_modules/ react-native field in package.json reso None of these files exist: * node_modules/aPackage/foo.js(.native.js|.js|.native.json|.json) - * node_modules/aPackage/foo.js/index(.native.js|.js|.native.json|.json) + * node_modules/aPackage/foo.js 1 | import foo from 'bar'; > 2 | import f from './foo.js'; | ^ @@ -176,7 +176,7 @@ exports[`linux platforms resolves platform-specific files 1`] = ` None of these files exist: * foo.js(.ios.js|.native.js|.js|.ios.json|.native.json|.json) - * foo.js/index(.ios.js|.native.js|.js|.ios.json|.native.json|.json) + * foo.js 1 | import foo from 'bar'; > 2 | import f from './foo.js'; | ^ @@ -188,7 +188,7 @@ exports[`linux relative paths fails when trying to implicitly require an extensi None of these files exist: * a.another(.native.js|.js|.native.json|.json) - * a.another/index(.native.js|.js|.native.json|.json) + * a.another 1 | import foo from 'bar'; > 2 | import root from './a.another'; | ^ @@ -200,7 +200,7 @@ exports[`linux relative paths with additional files included in the file map (wa None of these files exist: * a(.native.js|.js|.native.json|.json) - * a/index(.native.js|.js|.native.json|.json) + * a 1 | import foo from 'bar'; > 2 | import a from './a'; | ^ @@ -212,7 +212,7 @@ exports[`win32 assets checks asset extensions case insensitively 1`] = ` None of these files exist: * asset.PNG(.native.js|.js|.native.json|.json) - * asset.PNG\\\\index(.native.js|.js|.native.json|.json) + * asset.PNG 1 | import foo from 'bar'; > 2 | import a from './asset.PNG'; | ^ @@ -224,7 +224,7 @@ exports[`win32 assets resolves asset files with resolution suffixes (matching ex None of these files exist: * c@2x.png - * c@2x.png\\\\index(.native.js|.js|.native.json|.json) + * c@2x.png 1 | import foo from 'bar'; > 2 | import a from './c@2x.png'; | ^ @@ -236,7 +236,7 @@ exports[`win32 assets resolves asset files with resolution suffixes (matching si None of these files exist: * a@1.5x.png - * a@1.5x.png\\\\index(.native.js|.js|.native.json|.json) + * a@1.5x.png 1 | import foo from 'bar'; > 2 | import a from './a@1.5x.png'; | ^ @@ -248,7 +248,7 @@ exports[`win32 assets resolves custom asset extensions when overriding assetExts None of these files exist: * asset2.png(.native.js|.js|.native.json|.json) - * asset2.png\\\\index(.native.js|.js|.native.json|.json) + * asset2.png 1 | import foo from 'bar'; > 2 | import a from './asset2.png'; | ^ @@ -323,7 +323,7 @@ exports[`win32 packages in node_modules/ browser field in package.json resolves None of these files exist: * node_modules\\\\aPackage\\\\foo.js(.native.js|.js|.native.json|.json) - * node_modules\\\\aPackage\\\\foo.js\\\\index(.native.js|.js|.native.json|.json) + * node_modules\\\\aPackage\\\\foo.js 1 | import foo from 'bar'; > 2 | import f from './foo.js'; | ^ @@ -353,7 +353,7 @@ exports[`win32 packages in node_modules/ react-native field in package.json reso None of these files exist: * node_modules\\\\aPackage\\\\foo.js(.native.js|.js|.native.json|.json) - * node_modules\\\\aPackage\\\\foo.js\\\\index(.native.js|.js|.native.json|.json) + * node_modules\\\\aPackage\\\\foo.js 1 | import foo from 'bar'; > 2 | import f from './foo.js'; | ^ @@ -383,7 +383,7 @@ exports[`win32 platforms resolves platform-specific files 1`] = ` None of these files exist: * foo.js(.ios.js|.native.js|.js|.ios.json|.native.json|.json) - * foo.js\\\\index(.ios.js|.native.js|.js|.ios.json|.native.json|.json) + * foo.js 1 | import foo from 'bar'; > 2 | import f from './foo.js'; | ^ @@ -395,7 +395,7 @@ exports[`win32 relative paths fails when trying to implicitly require an extensi None of these files exist: * a.another(.native.js|.js|.native.json|.json) - * a.another\\\\index(.native.js|.js|.native.json|.json) + * a.another 1 | import foo from 'bar'; > 2 | import root from './a.another'; | ^ @@ -407,7 +407,7 @@ exports[`win32 relative paths with additional files included in the file map (wa None of these files exist: * a(.native.js|.js|.native.json|.json) - * a\\\\index(.native.js|.js|.native.json|.json) + * a 1 | import foo from 'bar'; > 2 | import a from './a'; | ^ diff --git a/packages/metro/src/integration_tests/__tests__/__snapshots__/build-errors-test.js.snap b/packages/metro/src/integration_tests/__tests__/__snapshots__/build-errors-test.js.snap index d51b6bbd02..4c83580cfc 100644 --- a/packages/metro/src/integration_tests/__tests__/__snapshots__/build-errors-test.js.snap +++ b/packages/metro/src/integration_tests/__tests__/__snapshots__/build-errors-test.js.snap @@ -5,7 +5,7 @@ Unable to resolve module ./does-not-exist from /cannot-resolve-require.js: None of these files exist: * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) - * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) + * build-errors/does-not-exist 13 | 14 | // $FlowExpectedError[cannot-resolve-module] > 15 | const DoesNotExist = require('./does-not-exist'); @@ -20,7 +20,7 @@ Unable to resolve module ./does-not-exist from /cannot-resolve-import.js: None of these files exist: * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) - * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) + * build-errors/does-not-exist 13 | 14 | // $FlowExpectedError[cannot-resolve-module] > 15 | import DoesNotExist from './does-not-exist'; @@ -35,7 +35,7 @@ Unable to resolve module ./does-not-exist from /inline-requires-cannot-reso None of these files exist: * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) - * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) + * build-errors/does-not-exist 13 | 14 | // $FlowExpectedError[cannot-resolve-module] > 15 | const DoesNotExist = require('./does-not-exist'); @@ -50,7 +50,7 @@ Unable to resolve module ./does-not-exist from /inline-requires-cannot-reso None of these files exist: * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) - * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) + * build-errors/does-not-exist 13 | 14 | // $FlowExpectedError[cannot-resolve-module] > 15 | import DoesNotExist from './does-not-exist'; @@ -65,7 +65,7 @@ Unable to resolve module ./does-not'"-exist from /cannot-resolve-multi-line None of these files exist: * build-errors/does-not'"-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) - * build-errors/does-not'"-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) + * build-errors/does-not'"-exist 14 | import type DoesNotExistT from './does-not/'"-exist'; 15 | > 16 | import { @@ -80,7 +80,7 @@ Unable to resolve module ./does-not'"-exist from /cannot-resolve-specifier- None of these files exist: * build-errors/does-not'"-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) - * build-errors/does-not'"-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) + * build-errors/does-not'"-exist 15 | 16 | // $FlowExpectedError[cannot-resolve-module] > 17 | import {DoesNotExist} from './does-not/'"-exist'; @@ -95,7 +95,7 @@ Unable to resolve module ./foo from /cannot-resolve-require-with-embedded-c None of these files exist: * build-errors/foo(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) - * build-errors/foo/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) + * build-errors/foo 13 | 14 | // $FlowExpectedError[cannot-resolve-module] > 15 | const DoesNotExist = require('./foo' /* ./foo */); @@ -110,7 +110,7 @@ Unable to resolve module ./does-not-exist from /cannot-resolve-multi-line-i None of these files exist: * build-errors/does-not-exist(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) - * build-errors/does-not-exist/index(.web.js|.native.js|.js|.web.jsx|.native.jsx|.jsx|.web.json|.native.json|.json|.web.ts|.native.ts|.ts|.web.tsx|.native.tsx|.tsx) + * build-errors/does-not-exist 25 | iiiiiiiiii, 26 | // $FlowExpectedError[cannot-resolve-module] > 27 | } from './does-not-exist';