-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Update node_modules_path to resolve symlinks to real paths #5085
Changes from 3 commits
178a2dd
81499cb
71c7fa3
047ea85
866dc5c
1b1bf90
a4622ec
f338a87
070ed48
0d14ebb
0d38947
d903193
6378e09
67f0155
ad713d4
6473273
f82fc37
3db5514
4cf6e5d
88776f9
39f7f07
2ec7a40
d45c6ff
1305408
47bd870
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
|
||
import type {Path} from 'types/Config'; | ||
import path from 'path'; | ||
import * as fs from 'fs'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed! I ended up not needing this as we used the utility function from |
||
|
||
type NodeModulesPathsOptions = {| | ||
moduleDirectory?: Array<string>, | ||
|
@@ -37,11 +38,16 @@ export default function nodeModulesPaths( | |
prefix = '\\\\'; | ||
} | ||
|
||
const paths = [basedirAbs]; | ||
let parsed = path.parse(basedirAbs); | ||
// we need to work with physical paths (i.e. follow symlinks to their | ||
// true location), which is what the node resolution algorithm does | ||
const physicalBasedir: string = fs.realpathSync(basedirAbs); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use the native realpath. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we've had issues with casing on windows using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks |
||
|
||
const paths = [physicalBasedir]; | ||
let parsed = path.parse(physicalBasedir); | ||
while (parsed.dir !== paths[paths.length - 1]) { | ||
paths.push(parsed.dir); | ||
parsed = path.parse(parsed.dir); | ||
const realParsedDir: string = fs.realpathSync(parsed.dir); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you need the type annotation here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes, we require this in our repositories (even in some places where it is a bit redundant). I'll go ahead and remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // obviously x is a number, so this annotation is redundant
const x = 123; // Nobody has any clue what the type of x is without
// using an analysis tool like VS Code
const x = getThing(); |
||
paths.push(realParsedDir); | ||
parsed = path.parse(realParsedDir); | ||
} | ||
|
||
const dirs = paths.reduce((dirs, aPath) => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to rephrase as "Update node module resolution algorithm to correctly handle symlinked paths"