-
Notifications
You must be signed in to change notification settings - Fork 1
Description
If you define a path of / on linux, true-case-path will return "". TBH, this is kind of an edge case; it's not often that somebody wants to check the case of a '/' 😆. I just happened to run across it because I was writing unit tests in windows for a function that uses this module, and the standard behavior for what I was testing is to use a folder right off the root of the drive (on windows). Instead of hard-coding a drive letter, I just used /folder.
Since this module will throw an error if it encounters a path segment that doesn't exist, it makes sense to only provide it the portion of a path that does exist. What winds up happening, if you try to define a path like /nonexistant-folder/some-other-folder/etc, and that first folder doesn't exist, / is what's used.
That is pretty much what is happening here in @microsoft/rushstack, except that it is using the thrown exception to determine that that final segment of the path doesn't exist (which results in many passes through true-case-path 😕).
This particular behavior is unfortunate, because that function is processing environment variables, which means it could be any arbitrary value. If the user defined an environment variable with a path like I described above, then it would have some unexpected results, because the path would be assumed to be relative to the cwd, instead of an absolute path.
I propose that a new option be included that will skip throwing an exception, and return something like this: '/case/checked/path/that/exists' + '/path/that/doesn't/exist'. That would also allow the function I mentioned to be refactored to remove the redundancy. It would definitely be possible to rewrite that function to handle this kind of behavior, but I think it would still wind up with extraneous filesystem calls that wouldn't really be necessary if we were able to make one pass through true-case-path.
On top of that, it seems like it would be safe to add an extra check here:
Lines 17 to 19 in ac422f9
| function getRelevantFilePathSegments(filePath) { | |
| return filePath.split(delimiter).filter((s) => s !== '') | |
| } |
like
.filter((s, i) => s !== '' || i == 0)But that's just how I would fix it 🤷♂
Thanks! 😄