Skip to content

Commit

Permalink
chore(cli): fix fast resolver (#27686)
Browse files Browse the repository at this point in the history
# Why

There was an issue where we attempted to match against the resolution
error in order to continue resolving, but the custom error didn't match
our matcher function. Now we should be able to use fast resolver in most
apps.

I've also switched the fs-extra methods over to fs which saves about
300ms in an app with 900 modules.

# Test Plan

- Tested manually.
- Tests should keep working.
- Added new tests for fragile check.

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
  • Loading branch information
EvanBacon and expo-bot committed Mar 15, 2024
1 parent 9a623ef commit c779c4e
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 95 deletions.
1 change: 1 addition & 0 deletions packages/@expo/cli/CHANGELOG.md
Expand Up @@ -16,6 +16,7 @@

### 🐛 Bug fixes

- Fix issue with fast resolver. ([#27686](https://github.com/expo/expo/pull/27686) by [@EvanBacon](https://github.com/EvanBacon))
- Fix using array syntax `(a,b)` with server output. ([#27462](https://github.com/expo/expo/pull/27462) by [@EvanBacon](https://github.com/EvanBacon))
- Prevent `console.log` statements from colliding with Metro logs. ([#27217](https://github.com/expo/expo/pull/27217) by [@EvanBacon](https://github.com/EvanBacon))
- Fix using dev server URL in development. ([#27213](https://github.com/expo/expo/pull/27213) by [@EvanBacon](https://github.com/EvanBacon))
Expand Down
4 changes: 2 additions & 2 deletions packages/@expo/cli/e2e/__tests__/export.test.ts
Expand Up @@ -58,7 +58,7 @@ describe('server', () => {
env: {
NODE_ENV: 'production',
TEST_BABEL_PRESET_EXPO_MODULE_ID: require.resolve('babel-preset-expo'),
EXPO_USE_FAST_RESOLVER: 'false',
EXPO_USE_FAST_RESOLVER: 'true',
},
});

Expand Down Expand Up @@ -201,7 +201,7 @@ describe('server', () => {
env: {
NODE_ENV: 'production',
TEST_BABEL_PRESET_EXPO_MODULE_ID: require.resolve('babel-preset-expo'),
EXPO_USE_FAST_RESOLVER: 'false',
EXPO_USE_FAST_RESOLVER: 'true',
},
}
);
Expand Down
2 changes: 1 addition & 1 deletion packages/@expo/cli/src/export/saveAssets.ts
Expand Up @@ -31,10 +31,10 @@ export type ExportAssetDescriptor = {
export type ExportAssetMap = Map<string, ExportAssetDescriptor>;

export async function persistMetroFilesAsync(files: ExportAssetMap, outputDir: string) {
fs.mkdirSync(path.join(outputDir), { recursive: true });
if (!files.size) {
return;
}
fs.mkdirSync(path.join(outputDir), { recursive: true });

// Test fixtures:
// Log.log(
Expand Down
21 changes: 13 additions & 8 deletions packages/@expo/cli/src/start/server/getStaticRenderFunctions.ts
Expand Up @@ -37,14 +37,19 @@ const debug = require('debug')('expo:start:server:node-renderer') as typeof cons
const cachedSourceMaps: Map<string, { url: string; map: string }> = new Map();

// Support unhandled rejections
require('source-map-support').install({
retrieveSourceMap(source: string) {
if (cachedSourceMaps.has(source)) {
return cachedSourceMaps.get(source);
}
return null;
},
});
// Detect if running in Bun

// @ts-expect-error: This is a global variable that is set by Bun.
if (!process.isBun) {
require('source-map-support').install({
retrieveSourceMap(source: string) {
if (cachedSourceMaps.has(source)) {
return cachedSourceMaps.get(source);
}
return null;
},
});
}

function wrapBundle(str: string) {
// Skip the metro runtime so debugging is a bit easier.
Expand Down
Expand Up @@ -3,7 +3,8 @@ import assert from 'assert';
import fs from 'fs';
import path from 'path';

import { createFastResolver } from '../createExpoMetroResolver';
import { createFastResolver, FailedToResolvePathError } from '../createExpoMetroResolver';
import { isFailedToResolvePathError } from '../metroErrors';

type SupportedContext = Parameters<ReturnType<typeof createFastResolver>>[0];

Expand Down Expand Up @@ -128,6 +129,13 @@ function resolveTo(
: null;
}

describe(isFailedToResolvePathError, () => {
it(`matches custom error`, () => {
const error = new FailedToResolvePathError('message');
expect(isFailedToResolvePathError(error)).toBe(true);
});
});

describe(createFastResolver, () => {
describe('node built-ins', () => {
it('shims node built-ins on non-server platforms', () => {
Expand Down
Expand Up @@ -13,10 +13,16 @@ import { isNodeExternal } from './externals';
import { formatFileCandidates } from './formatFileCandidates';
import { isServerEnvironment } from '../middleware/metroOptions';

class FailedToResolvePathError extends Error {}
export class FailedToResolvePathError extends Error {
// Added to ensure the error is matched by our tooling.
// TODO: Test that this matches `isFailedToResolvePathError`
candidates = {};
}

class ShimModuleError extends Error {}

const debug = require('debug')('expo:metro:resolve') as typeof console.log;

const realpathFS =
process.platform !== 'win32' && fs.realpathSync && typeof fs.realpathSync.native === 'function'
? fs.realpathSync.native
Expand All @@ -40,6 +46,7 @@ export function createFastResolver({
preserveSymlinks: boolean;
blockList: RegExp[];
}) {
debug('Creating with settings:', { preserveSymlinks, blockList });
const cachedExtensions: Map<string, readonly string[]> = new Map();

function getAdjustedExtensions({
Expand Down Expand Up @@ -110,17 +117,16 @@ export function createFastResolver({

let fp: string;

const conditions = context.unstable_enablePackageExports
? [
...new Set([
'default',
...context.unstable_conditionNames,
...(platform != null ? context.unstable_conditionsByPlatform[platform] ?? [] : []),
]),
]
: [];
try {
const conditions = context.unstable_enablePackageExports
? [
...new Set([
'default',
...context.unstable_conditionNames,
...(platform != null ? context.unstable_conditionsByPlatform[platform] ?? [] : []),
]),
]
: [];

fp = jestResolver(moduleName, {
blockList,
enablePackageExports: context.unstable_enablePackageExports,
Expand All @@ -129,8 +135,18 @@ export function createFastResolver({
extensions,
conditions,
realpathSync(file: string): string {
// @ts-expect-error: Missing on type.
const metroRealPath = context.unstable_getRealPath?.(file);
let metroRealPath: string | null = null;

try {
// @ts-expect-error: Missing on type.
metroRealPath = context.unstable_getRealPath?.(file);
} catch (error: any) {
// If invariant
if (error.message !== 'Unexpectedly escaped traversal') {
throw error;
}
}

if (metroRealPath == null && preserveSymlinks) {
return realpathSync(file);
}
Expand All @@ -156,13 +172,7 @@ export function createFastResolver({
// the app doesn't finish without it.
preserveSymlinks,
readPackageSync(readFileSync, pkgFile) {
return (
context.getPackage(pkgFile) ??
JSON.parse(
// @ts-expect-error
readFileSync(pkgfile)
)
);
return context.getPackage(pkgFile) ?? JSON.parse(fs.readFileSync(pkgFile, 'utf8'));
},
includeCoreModules: isServer,

Expand Down Expand Up @@ -206,6 +216,8 @@ export function createFastResolver({
};
}

debug({ moduleName, platform, conditions, isServer, preserveSymlinks }, context);

throw new FailedToResolvePathError(
'The module could not be resolved because no file or module matched the pattern:\n' +
` ${formatFileCandidates(
Expand Down
113 changes: 66 additions & 47 deletions packages/@expo/cli/src/start/server/metro/createJResolver.ts
Expand Up @@ -10,6 +10,7 @@
*/
import type { JSONObject as PackageJSON } from '@expo/json-file';
import assert from 'assert';
import fs from 'fs';
import { dirname, isAbsolute, resolve as pathResolve } from 'path';
import { sync as resolveSync, SyncOpts as UpstreamResolveOptions } from 'resolve';
import * as resolve from 'resolve.exports';
Expand Down Expand Up @@ -83,7 +84,10 @@ type ResolverOptions = {
| 'includeCoreModules'
>;

type UpstreamResolveOptionsWithConditions = UpstreamResolveOptions & ResolverOptions;
type UpstreamResolveOptionsWithConditions = UpstreamResolveOptions &
ResolverOptions & {
pathExists: (file: string) => boolean;
};

const defaultResolver = (
path: string,
Expand All @@ -109,6 +113,17 @@ const defaultResolver = (
}
return fileExistsSync(file);
},
pathExists(file) {
if (blockList.some((regex) => regex.test(file))) {
return false;
}
try {
fs.accessSync(path, fs.constants.F_OK);
return true; // File exists
} catch {
return false; // File doesn't exist
}
},
preserveSymlinks: options.preserveSymlinks,
defaultResolver,
};
Expand Down Expand Up @@ -138,61 +153,34 @@ function getPathInModule(path: string, options: UpstreamResolveOptionsWithCondit

let moduleName = segments.shift();

if (moduleName) {
if (moduleName.startsWith('@')) {
moduleName = `${moduleName}/${segments.shift()}`;
}

// Disable package exports for babel/runtime for https://github.com/facebook/metro/issues/984/
if (moduleName === '@babel/runtime') {
return path;
}

// self-reference
const closestPackageJson = findClosestPackageJson(options.basedir, options);
if (closestPackageJson) {
const pkg = options.readPackageSync!(options.readFileSync!, closestPackageJson);
assert(pkg, 'package.json should be read by `readPackageSync`');

if (pkg.name === moduleName) {
const resolved = resolve.exports(
pkg,
(segments.join('/') || '.') as resolve.Exports.Entry,
createResolveOptions(options.conditions)
);

if (resolved) {
return pathResolve(dirname(closestPackageJson), resolved[0]);
}

if (pkg.exports) {
throw new Error(
"`exports` exists, but no results - this is a bug in Expo CLI's Metro resolver. Please report an issue"
);
}
}
}
if (!moduleName) {
return path;
}

let packageJsonPath = '';
if (moduleName.startsWith('@')) {
moduleName = `${moduleName}/${segments.shift()}`;
}

try {
packageJsonPath = resolveSync(`${moduleName}/package.json`, options);
} catch {
// ignore if package.json cannot be found
}
// Disable package exports for babel/runtime for https://github.com/facebook/metro/issues/984/
if (moduleName === '@babel/runtime') {
return path;
}

if (packageJsonPath && options.isFile!(packageJsonPath)) {
const pkg = options.readPackageSync!(options.readFileSync!, packageJsonPath);
assert(pkg, 'package.json should be read by `readPackageSync`');
// self-reference
const closestPackageJson = findClosestPackageJson(options.basedir, options);
if (closestPackageJson) {
const pkg = options.readPackageSync!(options.readFileSync!, closestPackageJson);
assert(pkg, 'package.json should be read by `readPackageSync`');

if (pkg.name === moduleName) {
const resolved = resolve.exports(
pkg,
(segments.join('/') || '.') as resolve.Exports.Entry,
createResolveOptions(options.conditions)
);

if (resolved) {
return pathResolve(dirname(packageJsonPath), resolved[0]);
return pathResolve(dirname(closestPackageJson), resolved[0]);
}

if (pkg.exports) {
Expand All @@ -203,6 +191,37 @@ function getPathInModule(path: string, options: UpstreamResolveOptionsWithCondit
}
}

let packageJsonPath = '';

try {
packageJsonPath = resolveSync(`${moduleName}/package.json`, options);
} catch {
// ignore if package.json cannot be found
}

if (!packageJsonPath) {
return path;
}

const pkg = options.readPackageSync!(options.readFileSync!, packageJsonPath);
assert(pkg, 'package.json should be read by `readPackageSync`');

const resolved = resolve.exports(
pkg,
(segments.join('/') || '.') as resolve.Exports.Entry,
createResolveOptions(options.conditions)
);

if (resolved) {
return pathResolve(dirname(packageJsonPath), resolved[0]);
}

if (pkg.exports) {
throw new Error(
"`exports` exists, but no results - this is a bug in Expo CLI's Metro resolver. Please report an issue"
);
}

return path;
}

Expand All @@ -220,7 +239,7 @@ const shouldIgnoreRequestForExports = (path: string) => path.startsWith('.') ||
// https://github.com/lukeed/escalade/blob/2477005062cdbd8407afc90d3f48f4930354252b/src/sync.js
function findClosestPackageJson(
start: string,
options: UpstreamResolveOptions
options: UpstreamResolveOptionsWithConditions
): string | undefined {
let dir = pathResolve('.', start);
if (!options.isDirectory!(dir)) {
Expand All @@ -229,7 +248,7 @@ function findClosestPackageJson(

while (true) {
const pkgJsonFile = pathResolve(dir, './package.json');
const hasPackageJson = options.isFile!(pkgJsonFile);
const hasPackageJson = options.pathExists!(pkgJsonFile);

if (hasPackageJson) {
return pkgJsonFile;
Expand Down

0 comments on commit c779c4e

Please sign in to comment.