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

Improve pathUtils.absoluteToNormal correctness and performance #1210

Closed
wants to merge 3 commits 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
9 changes: 6 additions & 3 deletions packages/metro-file-map/src/crawlers/node/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type {
IgnoreMatcher,
} from '../../flow-types';

import * as fastPath from '../../lib/fast_path';
import {RootPathUtils} from '../../lib/RootPathUtils';
import hasNativeFindSupport from './hasNativeFindSupport';
import {spawn} from 'child_process';
import * as fs from 'graceful-fs';
Expand All @@ -37,6 +37,7 @@ function find(
): void {
const result: FileData = new Map();
let activeCalls = 0;
const pathUtils = new RootPathUtils(rootDir);

function search(directory: string): void {
activeCalls++;
Expand Down Expand Up @@ -71,7 +72,7 @@ function find(
if (!err && stat) {
const ext = path.extname(file).substr(1);
if (stat.isSymbolicLink() || extensions.includes(ext)) {
result.set(fastPath.relative(rootDir, file), [
result.set(pathUtils.absoluteToNormal(file), [
'',
stat.mtime.getTime(),
stat.size,
Expand Down Expand Up @@ -122,6 +123,8 @@ function findNative(
includeSymlinks ? '-o -type l ' : ''
})`;

const pathUtils = new RootPathUtils(rootDir);

const child = spawn('find', roots.concat(expression.split(' ')));
let stdout = '';
if (child.stdout == null) {
Expand All @@ -145,7 +148,7 @@ function findNative(
lines.forEach(path => {
fs.lstat(path, (err, stat) => {
if (!err && stat) {
result.set(fastPath.relative(rootDir, path), [
result.set(pathUtils.absoluteToNormal(path), [
'',
stat.mtime.getTime(),
stat.size,
Expand Down
10 changes: 6 additions & 4 deletions packages/metro-file-map/src/crawlers/watchman/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import type {
} from '../../flow-types';
import type {WatchmanQueryResponse, WatchmanWatchResponse} from 'fb-watchman';

import * as fastPath from '../../lib/fast_path';
import normalizePathSeparatorsToPosix from '../../lib/normalizePathSeparatorsToPosix';
import normalizePathSeparatorsToSystem from '../../lib/normalizePathSeparatorsToSystem';
import {RootPathUtils} from '../../lib/RootPathUtils';
import {planQuery} from './planQuery';
import invariant from 'invariant';
import * as path from 'path';
Expand Down Expand Up @@ -66,6 +66,7 @@ module.exports = async function watchmanCrawl({
abortSignal?.throwIfAborted();

const client = new watchman.Client();
const pathUtils = new RootPathUtils(rootDir);
abortSignal?.addEventListener('abort', () => client.end());

perfLogger?.point('watchmanCrawl_start');
Expand Down Expand Up @@ -177,6 +178,7 @@ module.exports = async function watchmanCrawl({
perfLogger?.point('watchmanCrawl/queryWatchmanForDirs_start');
const results = new Map<string, WatchmanQueryResponse>();
let isFresh = false;

await Promise.all(
Array.from(rootProjectDirMappings).map(
async ([root, {directoryFilters, watcher}], index) => {
Expand All @@ -189,7 +191,7 @@ module.exports = async function watchmanCrawl({
// By using scm queries, we can create the haste map on a different
// system and import it, transforming the clock into a local clock.
const since = previousState.clocks.get(
normalizePathSeparatorsToPosix(fastPath.relative(rootDir, root)),
normalizePathSeparatorsToPosix(pathUtils.absoluteToNormal(root)),
);

perfLogger?.annotate({
Expand Down Expand Up @@ -288,7 +290,7 @@ module.exports = async function watchmanCrawl({

for (const [watchRoot, response] of results) {
const fsRoot = normalizePathSeparatorsToSystem(watchRoot);
const relativeFsRoot = fastPath.relative(rootDir, fsRoot);
const relativeFsRoot = pathUtils.absoluteToNormal(fsRoot);
newClocks.set(
normalizePathSeparatorsToPosix(relativeFsRoot),
// Ensure we persist only the local clock.
Expand All @@ -300,7 +302,7 @@ module.exports = async function watchmanCrawl({
for (const fileData of response.files) {
const filePath =
fsRoot + path.sep + normalizePathSeparatorsToSystem(fileData.name);
const relativeFilePath = fastPath.relative(rootDir, filePath);
const relativeFilePath = pathUtils.absoluteToNormal(filePath);

if (!fileData.exists) {
if (!isFresh) {
Expand Down
18 changes: 8 additions & 10 deletions packages/metro-file-map/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ import H from './constants';
import getMockName from './getMockName';
import checkWatchmanCapabilities from './lib/checkWatchmanCapabilities';
import {DuplicateError} from './lib/DuplicateError';
import * as fastPath from './lib/fast_path';
import MockMapImpl from './lib/MockMap';
import MutableHasteMap from './lib/MutableHasteMap';
import normalizePathSeparatorsToSystem from './lib/normalizePathSeparatorsToSystem';
import {RootPathUtils} from './lib/RootPathUtils';
import TreeFS from './lib/TreeFS';
import {Watcher} from './Watcher';
import {worker} from './worker';
Expand Down Expand Up @@ -246,6 +246,7 @@ export default class FileMap extends EventEmitter {
_changeInterval: ?IntervalID;
_console: Console;
_options: InternalOptions;
_pathUtils: RootPathUtils;
_watcher: ?Watcher;
_worker: ?WorkerInterface;
_cacheManager: CacheManager;
Expand Down Expand Up @@ -330,6 +331,7 @@ export default class FileMap extends EventEmitter {
});

this._buildPromise = null;
this._pathUtils = new RootPathUtils(options.rootDir);
this._worker = null;
this._startupPerfLogger?.point('constructor_end');
this._crawlerAbortController = new AbortController();
Expand Down Expand Up @@ -526,7 +528,7 @@ export default class FileMap extends EventEmitter {
): ?Promise<void> {
const rootDir = this._options.rootDir;

const relativeFilePath = fastPath.relative(rootDir, filePath);
const relativeFilePath = this._pathUtils.absoluteToNormal(filePath);
const isSymlink = fileMetadata[H.SYMLINK] !== 0;

const computeSha1 =
Expand Down Expand Up @@ -628,7 +630,7 @@ export default class FileMap extends EventEmitter {
const existingMockPath = mockMap.get(mockPath);

if (existingMockPath != null) {
const secondMockPath = fastPath.relative(rootDir, filePath);
const secondMockPath = this._pathUtils.absoluteToNormal(filePath);
if (existingMockPath !== secondMockPath) {
const method = this._options.throwOnModuleCollision
? 'error'
Expand Down Expand Up @@ -706,10 +708,7 @@ export default class FileMap extends EventEmitter {
}

// SHA-1, if requested, should already be present thanks to the crawler.
const filePath = fastPath.resolve(
this._options.rootDir,
relativeFilePath,
);
const filePath = this._pathUtils.normalToAbsolute(relativeFilePath);
const maybePromise = this._processFile(
hasteMap,
mockMap,
Expand Down Expand Up @@ -879,8 +878,6 @@ export default class FileMap extends EventEmitter {
const hasWatchedExtension = (filePath: string) =>
this._options.extensions.some(ext => filePath.endsWith(ext));

const rootDir = this._options.rootDir;

let changeQueue: Promise<null | void> = Promise.resolve();
let nextEmit: ?{
eventsQueue: EventsQueue,
Expand Down Expand Up @@ -946,7 +943,8 @@ export default class FileMap extends EventEmitter {
return;
}

const relativeFilePath = fastPath.relative(rootDir, absoluteFilePath);
const relativeFilePath =
this._pathUtils.absoluteToNormal(absoluteFilePath);
const linkStats = fileSystem.linkStats(relativeFilePath);

// The file has been accessed, not modified. If the modified time is
Expand Down
6 changes: 4 additions & 2 deletions packages/metro-file-map/src/lib/MockMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,21 @@

import type {MockMap as IMockMap, Path, RawMockMap} from '../flow-types';

import {resolve} from './fast_path';
import {RootPathUtils} from './RootPathUtils';

export default class MockMap implements IMockMap {
+#raw: RawMockMap;
+#rootDir: Path;
+#pathUtils: RootPathUtils;

constructor({rawMockMap, rootDir}: {rawMockMap: RawMockMap, rootDir: Path}) {
this.#raw = rawMockMap;
this.#rootDir = rootDir;
this.#pathUtils = new RootPathUtils(rootDir);
}

getMockModule(name: string): ?Path {
const mockPath = this.#raw.get(name) || this.#raw.get(name + '/index');
return mockPath != null ? resolve(this.#rootDir, mockPath) : null;
return mockPath != null ? this.#pathUtils.normalToAbsolute(mockPath) : null;
}
}
10 changes: 6 additions & 4 deletions packages/metro-file-map/src/lib/MutableHasteMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import type {
import H from '../constants';
import {DuplicateError} from './DuplicateError';
import {DuplicateHasteCandidatesError} from './DuplicateHasteCandidatesError';
import * as fastPath from './fast_path';
import getPlatformExtension from './getPlatformExtension';
import {RootPathUtils} from './RootPathUtils';
import path from 'path';

const EMPTY_OBJ: $ReadOnly<{[string]: HasteMapItemMetaData}> = {};
Expand All @@ -45,13 +45,15 @@ export default class MutableHasteMap implements HasteMap {
#duplicates: DuplicatesIndex = new Map();

+#console: ?Console;
#throwOnModuleCollision: boolean;
+#pathUtils: RootPathUtils;
+#platforms: $ReadOnlySet<string>;
#throwOnModuleCollision: boolean;

constructor(options: HasteMapOptions) {
this.#console = options.console ?? null;
this.#platforms = options.platforms;
this.#rootDir = options.rootDir;
this.#pathUtils = new RootPathUtils(options.rootDir);
this.#throwOnModuleCollision = options.throwOnModuleCollision;
}

Expand Down Expand Up @@ -103,7 +105,7 @@ export default class MutableHasteMap implements HasteMap {
);
if (module && module[H.TYPE] === (type ?? H.MODULE)) {
const modulePath = module[H.PATH];
return modulePath && fastPath.resolve(this.#rootDir, modulePath);
return modulePath && this.#pathUtils.normalToAbsolute(modulePath);
}
return null;
}
Expand Down Expand Up @@ -186,7 +188,7 @@ export default class MutableHasteMap implements HasteMap {
const duplicates = new Map<string, number>();

for (const [relativePath, type] of relativePathSet) {
const duplicatePath = fastPath.resolve(this.#rootDir, relativePath);
const duplicatePath = this.#pathUtils.normalToAbsolute(relativePath);
duplicates.set(duplicatePath, type);
}

Expand Down
Loading