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

Resolver perf: Use directory existence to cut down candidate list (3/n) #1333

Closed
wants to merge 1 commit 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
18 changes: 13 additions & 5 deletions packages/metro-resolver/src/__tests__/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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: [],
},
}),
);
Expand All @@ -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);
Expand All @@ -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: {
Expand Down Expand Up @@ -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\`"
`);
});

Expand Down
23 changes: 21 additions & 2 deletions packages/metro-resolver/src/__tests__/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ type MockFileMap = $ReadOnly<{
export function createResolutionContext(
fileMap: MockFileMap = {},
): $Diff<ResolutionContext, {originModulePath: string}> {
const directorySet = new Set<string>();
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,
Expand All @@ -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 {
Expand Down
106 changes: 89 additions & 17 deletions packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,23 @@ 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') {
return result.resolution;
}
}

/**
* realModuleName is now a package specifier.
*/

const {disableHierarchicalLookup} = context;

const nodeModulesPaths = [];
Expand All @@ -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);
}
Expand All @@ -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]);
Expand All @@ -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
Expand Down Expand Up @@ -336,6 +395,19 @@ function resolvePackageEntryPoint(
packagePath: string,
platform: string | null,
): Result<Resolution, FileCandidates> {
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)) {
Expand Down
Loading
Loading