Skip to content

Fix Windows-style absolute paths in triple-slash directives#21205

Merged
uniqueiniquity merged 4 commits into
microsoft:masterfrom
uniqueiniquity:resolveTripleSlashReferencePaths
Jan 17, 2018
Merged

Fix Windows-style absolute paths in triple-slash directives#21205
uniqueiniquity merged 4 commits into
microsoft:masterfrom
uniqueiniquity:resolveTripleSlashReferencePaths

Conversation

@uniqueiniquity
Copy link
Copy Markdown
Contributor

@uniqueiniquity uniqueiniquity commented Jan 16, 2018

PR #19702 caused Windows-style absolute paths to be no longer understood as absolute paths.
This change appropriately normalizes paths in triple-slash directives so that they can be recognized correctly.

EDIT: This change now checks for backslashes in file paths to determine if a path is rooted. This was updated to cover more cases than just triple-slash directives.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Jan 17, 2018

You also mentioned another test for tsconfig paths, we should add that as well.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Jan 17, 2018

thanks!

@sheetalkamat
Copy link
Copy Markdown
Member

Instead of doing this shouldn't we be looking for usage of getRootLength to ensure it always passes paths that have normalized slashes ?

@uniqueiniquity
Copy link
Copy Markdown
Contributor Author

@sheetalkamat I discussed this with @mhegazy. We agreed that would definitely be a better approach but that it made sense to make this change to get back to the behavior we had before #19702.

I've started looking into what you suggested and it quickly became unclear; for example, we use the Path type to represent normalized paths, but the toPath function uses getRootLength to determine if a string is a normalized path yet or not.

@uniqueiniquity uniqueiniquity merged commit 8f6c516 into microsoft:master Jan 17, 2018
@uniqueiniquity uniqueiniquity deleted the resolveTripleSlashReferencePaths branch January 25, 2018 00:06
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants