diff --git a/packages/metro-file-map/src/crawlers/node/index.js b/packages/metro-file-map/src/crawlers/node/index.js index e7aaabe7a7..7c5427403c 100644 --- a/packages/metro-file-map/src/crawlers/node/index.js +++ b/packages/metro-file-map/src/crawlers/node/index.js @@ -42,7 +42,19 @@ function find( let activeCalls = 0; const pathUtils = new RootPathUtils(rootDir); - function search(directory: string): void { + const exts = extensions.reduce( + (acc, ext) => { + acc[ext] = true; + return acc; + }, + {} as {[string]: ?true}, + ); + + function search( + directory: string, + dirNormal: string, + isWithinRoot: boolean, + ): void { activeCalls++; fs.readdir(directory, {withFileTypes: true}, (err, entries) => { activeCalls--; @@ -51,46 +63,55 @@ function find( `Error "${err.code ?? err.message}" reading contents of "${directory}", skipping. Add this directory to your ignore list to exclude it.`, ); } else { - entries.forEach((entry: fs.Dirent) => { - const file = path.join(directory, entry.name.toString()); - - if (ignore(file)) { - return; + for (let idx = 0; idx < entries.length; idx++) { + const entry = entries[idx]; + const name = entry.name.toString(); + const file = directory + path.sep + name; + + const isSymbolicLink = entry.isSymbolicLink(); + if (ignore(file) || (!includeSymlinks && isSymbolicLink)) { + continue; } - if (entry.isSymbolicLink() && !includeSymlinks) { - return; - } + // Deriving a normal path above the root dir requires slicing off an up-fragment + // then checking if the target matches the next segment of the root dir. It's therefore + // easier to fall back to `pathUtils.absoluteToNormal` + const childNormal = !isWithinRoot + ? pathUtils.absoluteToNormal(file) + : dirNormal === '' + ? name + : dirNormal + path.sep + name; if (entry.isDirectory()) { - search(file); - return; + search(file, childNormal, isWithinRoot || childNormal === ''); + continue; } - activeCalls++; + const ext = path.extname(file).substr(1); + if (!isSymbolicLink && !exts[ext]) { + continue; + } + activeCalls++; fs.lstat(file, (err, stat) => { activeCalls--; if (!err && stat) { - const ext = path.extname(file).substr(1); - if (stat.isSymbolicLink() || extensions.includes(ext)) { - result.set(pathUtils.absoluteToNormal(file), [ - stat.mtime.getTime(), - stat.size, - 0, - null, - stat.isSymbolicLink() ? 1 : 0, - null, - ]); - } + result.set(childNormal, [ + stat.mtime.getTime(), + stat.size, + 0, + null, + isSymbolicLink ? 1 : 0, + null, + ]); } if (activeCalls === 0) { callback(result); } }); - }); + } } if (activeCalls === 0) { @@ -100,7 +121,11 @@ function find( } if (roots.length > 0) { - roots.forEach(search); + for (const root of roots) { + const rootNormal = pathUtils.absoluteToNormal(root); + const isWithinRoot = !rootNormal.startsWith('..' + path.sep); + search(root, rootNormal, isWithinRoot); + } } else { callback(result); } @@ -149,7 +174,7 @@ function findNative( if (!count) { callback(new Map()); } else { - lines.forEach(path => { + for (const path of lines) { fs.lstat(path, (err, stat) => { if (!err && stat) { result.set(pathUtils.absoluteToNormal(path), [ @@ -165,7 +190,7 @@ function findNative( callback(result); } }); - }); + } } }); } @@ -197,43 +222,25 @@ export default async function nodeCrawl( debug('Using system find: %s', useNativeFind); - return new Promise((resolve, reject) => { - const callback: Callback = fileData => { - const difference = previousState.fileSystem.getDifference(fileData, { - subpath, - }); + const crawlFn = useNativeFind ? findNative : find; + const fileData = await new Promise(resolve => { + crawlFn( + roots, + extensions, + ignore, + includeSymlinks, + rootDir, + console, + resolve, + ); + }); - perfLogger?.point('nodeCrawl_end'); + abortSignal?.throwIfAborted(); - try { - // TODO: Use AbortSignal.reason directly when Flow supports it - abortSignal?.throwIfAborted(); - } catch (e) { - reject(e); - } - resolve(difference); - }; - - if (useNativeFind) { - findNative( - roots, - extensions, - ignore, - includeSymlinks, - rootDir, - console, - callback, - ); - } else { - find( - roots, - extensions, - ignore, - includeSymlinks, - rootDir, - console, - callback, - ); - } + const difference = previousState.fileSystem.getDifference(fileData, { + subpath, }); + + perfLogger?.point('nodeCrawl_end'); + return difference; } diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index 113f85be1d..387057e865 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -49,6 +49,7 @@ import {FileProcessor} from './lib/FileProcessor'; import {FileSystemChangeAggregator} from './lib/FileSystemChangeAggregator'; import normalizePathSeparatorsToPosix from './lib/normalizePathSeparatorsToPosix'; import normalizePathSeparatorsToSystem from './lib/normalizePathSeparatorsToSystem'; +import removeOverlappingRoots from './lib/removeOverlappingRoots'; import {RootPathUtils} from './lib/RootPathUtils'; import TreeFS from './lib/TreeFS'; import {Watcher} from './Watcher'; @@ -323,7 +324,7 @@ export default class FileMap extends EventEmitter { plugins, retainAllFiles: options.retainAllFiles, rootDir: options.rootDir, - roots: Array.from(new Set(options.roots)), + roots: removeOverlappingRoots(options.roots), }; this.#options = { diff --git a/packages/metro-file-map/src/lib/__tests__/removeOverlappingRoots-test.js b/packages/metro-file-map/src/lib/__tests__/removeOverlappingRoots-test.js new file mode 100644 index 0000000000..084dd41e21 --- /dev/null +++ b/packages/metro-file-map/src/lib/__tests__/removeOverlappingRoots-test.js @@ -0,0 +1,119 @@ +/** + * 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. + * + * @format + * @flow strict + * @oncall react_native + */ + +let mockPathModule; +jest.mock('path', () => mockPathModule); + +describe.each([['win32'], ['posix']])( + 'removeOverlappingRoots on %s', + platform => { + // Convenience function to write paths with posix separators but convert them + // to system separators + const p: string => string = filePath => + platform === 'win32' + ? filePath.replace(/\//g, '\\').replace(/^\\/, 'C:\\') + : filePath; + + let removeOverlappingRoots; + + beforeEach(() => { + jest.resetModules(); + mockPathModule = jest.requireActual<{}>('path')[platform]; + removeOverlappingRoots = require('../removeOverlappingRoots').default; + }); + + test('returns empty array for empty input', () => { + expect(removeOverlappingRoots([])).toEqual([]); + }); + + test('returns single root unchanged', () => { + expect(removeOverlappingRoots([p('/a/b')])).toEqual([p('/a/b')]); + }); + + test('sorts roots', () => { + expect(removeOverlappingRoots([p('/b'), p('/a')])).toEqual([ + p('/a'), + p('/b'), + ]); + }); + + test('removes exact duplicates', () => { + expect(removeOverlappingRoots([p('/a'), p('/b'), p('/a')])).toEqual([ + p('/a'), + p('/b'), + ]); + }); + + test('removes a subdirectory of another root', () => { + expect(removeOverlappingRoots([p('/a/b'), p('/a/b/c')])).toEqual([ + p('/a/b'), + ]); + }); + + test('removes deeply nested subdirectories', () => { + expect(removeOverlappingRoots([p('/a'), p('/a/b'), p('/a/b/c')])).toEqual( + [p('/a')], + ); + }); + + test('keeps sibling directories', () => { + expect(removeOverlappingRoots([p('/a/b'), p('/a/c')])).toEqual([ + p('/a/b'), + p('/a/c'), + ]); + }); + + test('does not treat a path-prefix as a parent (e.g. /a/b vs /a/b-foo)', () => { + expect(removeOverlappingRoots([p('/a/b'), p('/a/b-foo')])).toEqual([ + p('/a/b-foo'), + p('/a/b'), + ]); + }); + + test('filters subdirectories even when interleaved with non-children', () => { + expect( + removeOverlappingRoots([p('/a/b/c'), p('/a/b-foo'), p('/a/b')]), + ).toEqual([p('/a/b-foo'), p('/a/b')]); + }); + + test('shorter parent always sorts before longer child', () => { + expect( + removeOverlappingRoots([p('/a/long/nested/path'), p('/a')]), + ).toEqual([p('/a')]); + }); + + test('handles a mix of duplicates, subdirectories, and siblings', () => { + expect( + removeOverlappingRoots([ + p('/project/src'), + p('/project/lib'), + p('/project/src/utils'), + p('/project/src'), + p('/project/lib/internal'), + p('/other'), + ]), + ).toEqual([p('/other'), p('/project/lib'), p('/project/src')]); + }); + + test('resolves paths (normalizes trailing slashes and ..)', () => { + expect(removeOverlappingRoots([p('/a/b/'), p('/a/c/../d')])).toEqual([ + p('/a/b'), + p('/a/d'), + ]); + }); + + test('resolves paths before deduplicating', () => { + expect(removeOverlappingRoots([p('/a/b'), p('/a/b/')])).toEqual([ + p('/a/b'), + ]); + }); + }, +); diff --git a/packages/metro-file-map/src/lib/removeOverlappingRoots.js b/packages/metro-file-map/src/lib/removeOverlappingRoots.js new file mode 100644 index 0000000000..5a6e1b7576 --- /dev/null +++ b/packages/metro-file-map/src/lib/removeOverlappingRoots.js @@ -0,0 +1,35 @@ +/** + * 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 + * @format + */ + +import path from 'path'; + +export default function removeOverlappingRoots( + roots: ReadonlyArray, +): ReadonlyArray { + const sorted = roots + .map(r => path.resolve(r)) + .sort((a, b) => { + const aRoot = a + path.sep; + const bRoot = b + path.sep; + return aRoot < bRoot ? -1 : aRoot > bRoot ? 1 : 0; + }); + if (sorted.length === 0) { + return sorted; + } + const result = [sorted[0]]; + for (let i = 1; i < sorted.length; i++) { + const rootPath = sorted[i] + path.sep; + const prevPath = result[result.length - 1] + path.sep; + if (!rootPath.startsWith(prevPath)) { + result.push(sorted[i]); + } + } + return result; +} diff --git a/packages/metro-file-map/types/lib/removeOverlappingRoots.d.ts b/packages/metro-file-map/types/lib/removeOverlappingRoots.d.ts new file mode 100644 index 0000000000..014449a14e --- /dev/null +++ b/packages/metro-file-map/types/lib/removeOverlappingRoots.d.ts @@ -0,0 +1,20 @@ +/** + * 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. + * + * @noformat + * @generated SignedSource<> + * + * This file was translated from Flow by scripts/generateTypeScriptDefinitions.js + * Original file: packages/metro-file-map/src/lib/removeOverlappingRoots.js + * To regenerate, run: + * js1 build metro-ts-defs (internal) OR + * yarn run build-ts-defs (OSS) + */ + +declare function removeOverlappingRoots( + roots: ReadonlyArray, +): ReadonlyArray; +export default removeOverlappingRoots;