| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Use realpathSync.native on case-insensitive file systems #44966
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
Conversation
typescript-bot
added
the
For Milestone Bug
|
If we don't like the API change, I have another version that threads a parameter through all the places that need it. I still need to do some manual testing around scenarios like this. Edit: confirmed that I regard this as fairly risky, so my vote would be to defer it until 4.5. |
|
@typescript-bot pack this |
|
I marked this as a draft because I don't want it to get merged by accident, but it's ready for review. |
|
Hey @amcasey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
| @@ -280,6 +280,11 @@ namespace ts { | |||
| } | |||
| const nodeModulesAtTypes = combinePaths("node_modules", "@types"); | |||
|
|
|||
| function arePathsEqual(path1: string, path2: string, host: ModuleResolutionHost): boolean { | |||
| const useCaseSensitiveFileNames = typeof host.useCaseSensitiveFileNames === "function" ? host.useCaseSensitiveFileNames() : host.useCaseSensitiveFileNames; | |||
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.
Do we expect the host to change its answer over time? Seems like you can avoid calling this over and over for every set of paths, right?
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.
I also think it's ridiculous that some hosts expose it as a function, but I'm not sure how comfortable I am with ignoring that.
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.
I suppose assuming that it won't change over time is no more outrageous than assuming that every implementation of realpath preserves lettercase when possible.
...so that path comparisons can use it during module resolution.
|
Force push is a rebase. Only notable merge conflict was undeleting the function Andrew mentioned above. |
…4966) * Make getSourceOfProjectReferenceRedirect take a Path * Add useCaseSensitiveFileNames to ModuleResolutionHost ...so that path comparisons can use it during module resolution. * Re-enable realpathSync.native for case-insensitive file systems
...chiefly, on Windows.
After re-reviewing all usages of realpath in the codebase, I believe there were three places where the input and output of realpath were being compared without specifically taking case into account. Fixing the two in module resolution as a little involved, since they didn't formerly have access to the case-sensitivity flag.
Fixes #43105