fix: resolve real path for case insensitive systems #4935
Conversation
|
💖 Thanks for opening this pull request! 💖 |
|
|
||
| async function getRealPath (path: string) { | ||
| return new Promise<string>((resolve) => { | ||
| fs.realpath.native(path, function (err, resolvedPath) { |
There was a problem hiding this comment.
Why shoud fs.realpath.native be used instead of fs.realpath?
There was a problem hiding this comment.
fs.realpath.native return the actual casing of a path as it is on disk https://github.com/nodejs/node/pull/15776/files#diff-05bc37150ff4e7aa51dc7a28c71164a9e715f45e323ac9a5c67e061821aad3d2R8, fs.realpath is resolving .., . and sym links
There was a problem hiding this comment.
in that case it is better to add a comment about it because in the Node.js docs this information is not present.
There was a problem hiding this comment.
@zkochan I've added comments on getRealPath
|
@zkochan I fixed test for case-sensitive system. I skipped them, because it is not possible to open the path |
mdogadailo
force-pushed
the
bugfix/fwd-case-insensitive-systems
branch
from
e58ccd4
to
c5e930e
Compare
|
Run |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
For case insensitive systems (Windows)
findWorkspaceDirwill return the path to the module as it was called (because ofcwd) not the real path.It leads to an issue when
getImporterscan't find the the manifest (#4904).Can be reproduces if
pnpmwas called from "c:\code\project" but the real path is "C:\Code\Project".getImporterswill find allmanifestsByPathas "C:\Code\Project...", butprefixeswill use workspace root "c:\code\project...", and it can't match them.It is possible to try true-case-path but has some vulnerable deps.