Skip to content

metro-file-map: Check hierarchicalLookup candidates from their own nodes #1946

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.

Sign up for GitHub

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

Draft
robhogan wants to merge 1 commit into from
Draft

metro-file-map: Check hierarchicalLookup candidates from their own nodes #1946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
110 changes: 86 additions & 24 deletions packages/metro-file-map/src/lib/TreeFS.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,9 +581,9 @@ export default class TreeFS implements MutableFileSystem {
// traversal, useful when adding files. Will throw if an expected
// directory is already present as a file.
makeDirectories?: boolean,
startPathIdx?: number,
startNode?: DirectoryNode,
start?: {
// Resume traversal from a known directory node, whose normal path is
// the prefix of the requested path ending at pathIdx - 1.
start?: ?{
ancestorOfRootIdx: ?number,
node: DirectoryNode,
pathIdx: number,
Expand Down Expand Up @@ -844,6 +844,10 @@ export default class TreeFS implements MutableFileSystem {
* X = dirname(X)
* while X !== dirname(X)
*
* Each candidate is checked from its own node: a subpath that is a single
* path segment, such as `package.json`, with one map lookup, and a deeper
* subpath by traversing only the subpath.
*
* If `observations` is given, records the canonical paths this result
* depends upon.
*
Expand All @@ -863,6 +867,12 @@ export default class TreeFS implements MutableFileSystem {
absolutePath: string,
containerRelativePath: string,
} {
// Only a single path segment can be looked up on a candidate's own node.
const isBasename =
subpath !== '' &&
subpath !== '.' &&
subpath !== '..' &&
!subpath.includes(path.sep);
const ancestorsOfInput: Array<{
ancestorOfRootIdx: ?number,
node: DirectoryNode,
Expand All @@ -881,7 +891,11 @@ export default class TreeFS implements MutableFileSystem {
subpath,
opts.subpathType,
observations,
null,
{
ancestorOfRootIdx: closestLookup.ancestorOfRootIdx,
node: closestLookup.node,
},
isBasename,
);
if (maybeAbsolutePathMatch != null) {
return {
Expand Down Expand Up @@ -954,14 +968,8 @@ export default class TreeFS implements MutableFileSystem {
subpath,
opts.subpathType,
observations,
{
ancestorOfRootIdx: candidate.ancestorOfRootIdx,
node: candidate.node,
pathIdx:
candidate.normalPath.length > 0
? candidate.normalPath.length + 1
: 0,
},
candidate,
isBasename,
);
if (maybeAbsolutePathMatch != null) {
// Determine the input path relative to the current candidate. Note
Expand Down Expand Up @@ -1001,7 +1009,11 @@ export default class TreeFS implements MutableFileSystem {
subpath,
opts.subpathType,
observations,
null,
{
ancestorOfRootIdx: commonRootDepth + depthBelowCommonRoot,
node: nextNode,
},
isBasename,
);
if (maybeAbsolutePathMatch != null) {
const rootDirParts = this.#pathUtils.getParts();
Expand Down Expand Up @@ -1030,24 +1042,74 @@ export default class TreeFS implements MutableFileSystem {
return null;
}

/**
* Whether the real directory at `normalCandidatePath` has `subpath` of the
* given type, returning the absolute real path of the match.
*
* `candidate` is the directory node at `normalCandidatePath`. When
* `isBasename` and the candidate is the root or a descendant of it, the
* child is checked with one map lookup. An ancestor of the root is
* traversed instead, because its node does not hold the segment leading
* back towards the root. Otherwise, traversal starts from the candidate.
*/
#checkCandidateHasSubpath(
normalCandidatePath: string,
subpath: string,
subpathType: 'f' | 'd',
observations: ?Observations,
start: ?{
ancestorOfRootIdx: ?number,
node: DirectoryNode,
pathIdx: number,
candidate: {
readonly ancestorOfRootIdx: ?number,
readonly node: DirectoryNode,
...
},
isBasename: boolean,
): ?string {
const lookupResult = this.#lookupByNormalPath(
this.#pathUtils.joinNormalToRelative(normalCandidatePath, subpath)
.normalPath,
{
observations,
},
);
if (
isBasename &&
(candidate.ancestorOfRootIdx == null || candidate.ancestorOfRootIdx === 0)
) {
const childNode = candidate.node.get(subpath);
// A symlink must be traversed to learn its type and real path, so only
// a missing child, a directory or a regular file is answered here.
if (
childNode == null ||
isDirectory(childNode) ||
isRegularFile(childNode)
) {
const isMatch =
childNode != null && isDirectory(childNode) === (subpathType === 'd');
if (!isMatch && observations == null) {
return null;
}
const childNormalPath =
normalCandidatePath === ''
? subpath
: normalCandidatePath + path.sep + subpath;
if (observations) {
observations.existence.add(childNormalPath);
}
return isMatch
? this.#pathUtils.normalToAbsolute(childNormalPath)
: null;
}
}
const {normalPath: normalSubpath, collapsedSegments} =
this.#pathUtils.joinNormalToRelative(normalCandidatePath, subpath);
const lookupResult = this.#lookupByNormalPath(normalSubpath, {
observations,
// Traverse only the subpath, starting from the candidate node. If the
// join collapsed segments (e.g. '..' + 'root/package.json'), the
// candidate path is no longer a prefix, so start from the root.
start:
collapsedSegments === 0
? {
ancestorOfRootIdx: candidate.ancestorOfRootIdx,
node: candidate.node,
pathIdx:
normalCandidatePath === '' ? 0 : normalCandidatePath.length + 1,
}
: null,
});
if (
lookupResult.exists &&
// Should be a Map iff subpathType is directory
Expand Down
65 changes: 63 additions & 2 deletions packages/metro-file-map/src/lib/__tests__/TreeFS-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,67 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
});
});

test('finds the closest directory with a single-segment subpath', () => {
const observations = emptyObservations();
expect(
tfs.hierarchicalLookup(
p('/A/B/C/a/n_m/pkg/subpath/deep'),
'n_m',
{breakOnSegment: null, subpathType: 'd'},
observations,
),
).toEqual({
absolutePath: p('/A/B/C/a/n_m/pkg/n_m'),
containerRelativePath: p('subpath/deep'),
});
expect(observations).toEqual({
content: new Set(),
existence: new Set([
p('a/n_m/pkg/subpath/deep/n_m'),
p('a/n_m/pkg/subpath/n_m'),
p('a/n_m/pkg/n_m'),
]),
});
});

test('finds the closest match for a deep subpath', () => {
const observations = emptyObservations();
expect(
tfs.hierarchicalLookup(
p('/A/B/C/a/b/c'),
p('n_m/pkg'),
{breakOnSegment: null, subpathType: 'd'},
observations,
),
).toEqual({
absolutePath: p('/A/B/C/a/n_m/pkg'),
containerRelativePath: p('b/c'),
});
expect(observations).toEqual({
content: new Set(),
existence: new Set([p('a/b/c/n_m'), p('a/b/n_m'), p('a/n_m/pkg')]),
});
});

test('finds a subpath re-entering the root from an ancestor of the root', () => {
const observations = emptyObservations();
expect(
tfs.hierarchicalLookup(
p('/A/foo'),
p('B/C/a/package.json'),
{breakOnSegment: null, subpathType: 'f'},
observations,
),
).toEqual({
absolutePath: p('/A/B/C/a/package.json'),
containerRelativePath: 'foo',
});
expect(observations).toEqual({
content: new Set(),
existence: new Set([p('../../foo'), p('a/package.json')]),
});
});

// The fourth column is paths, besides the match itself, whose addition or
// removal changes the result, the fifth those whose modification does too -
// the symlinks traversed.
Expand Down Expand Up @@ -662,6 +723,8 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
) => {
const pathMap = (normalPosixPath: string) =>
mockPathModule.resolve(p('/A/B/C'), p(normalPosixPath));
const toCanonical = (posixPath: string) =>
canonicalTo(p('/A/B/C'))(pathMap(posixPath));
const observations = emptyObservations();
expect(
tfs.hierarchicalLookup(
Expand All @@ -681,8 +744,6 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
containerRelativePath: p(expectedRelativeSubpath),
},
);
const toCanonical = (posixPath: string) =>
canonicalTo(p('/A/B/C'))(pathMap(posixPath));
expect(observations).toEqual({
content: new Set(expectedContent.map(toCanonical)),
existence: new Set(
Expand Down
Loading