Call toPath on sourceRoot to ensure it is always absolute#25838
Conversation
| //// member: string; | ||
| //// /*2*/methodName(propName: SomeType): void {} | ||
| //// otherMethod() { | ||
| //// if (Math.random() > 0.5) { |
There was a problem hiding this comment.
Could you try to cut the repro down to size?
There was a problem hiding this comment.
I mean, I didn't want to since it is just a copy of the other test with a relative sourceroot patched in. :3
| export function decode(host: SourceMapDecodeHost, mapPath: string, map: SourceMapData, program?: Program, fallbackCache = createSourceFileLikeCache(host)): SourceMapper { | ||
| const currentDirectory = getDirectoryPath(mapPath); | ||
| const sourceRoot = map.sourceRoot || currentDirectory; | ||
| const sourceRoot = map.sourceRoot ? toPath(map.sourceRoot, currentDirectory, p => host.getCanonicalFileName(p)) : currentDirectory; |
There was a problem hiding this comment.
Do you need to use getNormalizedAbsolutePath instead to avoid messing with the casing when not needed?
There was a problem hiding this comment.
Within this file, sourceRoot is passed into comparePaths, and as a base for toPath, so in all places it would just be canonicalized later anyway. By canonicalizing it early, hopefully work is repeated less.
| export function decode(host: SourceMapDecodeHost, mapPath: string, map: SourceMapData, program?: Program, fallbackCache = createSourceFileLikeCache(host)): SourceMapper { | ||
| const currentDirectory = getDirectoryPath(mapPath); | ||
| const sourceRoot = map.sourceRoot || currentDirectory; | ||
| const sourceRoot = map.sourceRoot ? getNormalizedAbsolutePath(map.sourceRoot, currentDirectory) : currentDirectory; |
There was a problem hiding this comment.
Do you need to handle if the sourceRoot specified is URL ?
There was a problem hiding this comment.
getNormalizedAbsolutePath already passes along URLs just fine (it preserves the input root if it is rooted), as will the rest of the path handling in the sourcemap code. The language service will likely summarily reject the mapped location, however, since https://github.com/Microsoft/TypeScript/tree/master/src/compiler/checker.ts (for example) won't exist on disk (though I imagine a host implementation could exist that returns results for such paths). UNC paths might in theory work fine, though.... those return normal results via fileExists and similar APIs on windows, right?
In any case: No, I don't think so - all the path normalizing/building stuff already is.
|
@weswigham can you please port to release-3.0 as well. |
…25838) * Call toPath on sourceRoot to ensure it is always absolute * Leave canonicalization to avoid a Path/string union
Call toPath on sourceRoot to ensure it is always absolute (#25838)
Fixes #25322