Skip to content

Commit

Permalink
Resolver: allow drive-less absolute paths on Windows
Browse files Browse the repository at this point in the history
Summary:
Fix a mostly Meta-specific edge case in Windows resolution. When using absolute paths without a drive letter on Windows, eg `\my\project`, don't use `path.resolve` to normalise to system separators, as this has the unwanted side-effect of prepending the the CWD's drive letter, even when the input is known to be absolute (according to `path.isAbsolute`):

On a Windows machine:
```
C:\> node -p "path.isAbsolute('/my/project')"
true

C:\> node -p "path.isAbsolute('\\my\\project')"
true

C:\> node -p "path.resolve('/my/project')"
C:\my\project
```

This preserves drive-less original paths. It also makes absolute path handling slightly stricter on Windows by not fully normalising the input (e.g., not normalising `C:\foo/../foo/.//bar` to `C:\foo\bar`, but instead using a simple `replaceAll`), which brings it into line with posix path handling - I'm considering this a "fix" rather than breaking, as behaviour for poorly-formed inputs is undefined.

Changelog:
```
 - **[Fix]**: Treat absolute path specifiers on Windows with the same strictness as posix, and allow absolute paths without drive letters.
```

Reviewed By: huntie

Differential Revision: D63175565

fbshipit-source-id: 35ab129f4846579dfe6ae8860d6d49b09cadc4e8
  • Loading branch information
robhogan authored and facebook-github-bot committed Sep 23, 2024
1 parent 398939a commit 8f7e000
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 12 deletions.
4 changes: 3 additions & 1 deletion packages/metro-resolver/src/PackageResolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ export function redirectModulePath(
// BRITTLE ASSUMPTION: This is always treated as a package-relative path
// and is converted back, even if the redirected path is a specifier
// referring to another package.
redirectedPath = path.resolve(containingPackage.rootPath, redirectedPath);
redirectedPath = path.isAbsolute(redirectedPath)
? path.normalize(redirectedPath)
: path.join(containingPackage.rootPath, redirectedPath);
}
} else {
// Otherwise, `modulePath` may be an unprefixed relative path or a bare
Expand Down
15 changes: 4 additions & 11 deletions packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,11 @@ function resolveModulePath(
toModuleName: string,
platform: string | null,
): Result<Resolution, FileAndDirCandidates> {
// System-separated absolute path
const modulePath = path.isAbsolute(toModuleName)
? resolveWindowsPath(toModuleName)
? path.sep === '/'
? toModuleName
: toModuleName.replaceAll('/', '\\')
: path.join(path.dirname(context.originModulePath), toModuleName);
const redirectedPath = context.redirectModulePath(modulePath);
if (redirectedPath === false) {
Expand Down Expand Up @@ -584,16 +587,6 @@ function resolveSourceFileForExt(
return null;
}

// HasteFS stores paths with backslashes on Windows, this ensures the path is in
// the proper format. Will also add drive letter if not present so `/root` will
// resolve to `C:\root`. Noop on other platforms.
function resolveWindowsPath(modulePath: string) {
if (path.sep !== '\\') {
return modulePath;
}
return path.resolve(modulePath);
}

function isRelativeImport(filePath: string) {
return /^[.][.]?(?:[/]|$)/.test(filePath);
}
Expand Down

0 comments on commit 8f7e000

Please sign in to comment.