Skip to content

Commit

Permalink
fix(compiler-cli): only pass canonical genfile paths to compiler host (
Browse files Browse the repository at this point in the history
…#27062)

In a more specific scenario: Considering people use a custom TypeScript compiler host with `NGC`, they _could_ expect only posix paths in methods like `writeFile`. This at first glance sounds like a trivial issue that should be just fixed by the actual compiler host, but usually TypeScript internal API's just pass around posix normalized paths, and therefore it would be good to follow the same standards when passing JSON genfiles to the `CompilerHost`.

For normal TypeScript files (and TS genfiles), this is already consistent because those will be handled by the actual TypeScript `Program` (see `emitCallback`).

PR Close #27062
  • Loading branch information
devversion authored and AndrewKushnir committed Nov 13, 2018
1 parent 6b1780d commit 0ada23a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 10 deletions.
20 changes: 12 additions & 8 deletions packages/compiler-cli/src/transformers/program.ts
Expand Up @@ -974,10 +974,8 @@ function normalizeSeparators(path: string): string {
* TODO(tbosch): talk to the TypeScript team to expose their logic for calculating the `rootDir`
* if none was specified.
*
* Note: This function works on normalized paths from typescript.
*
* @param outDir
* @param outSrcMappings
* Note: This function works on normalized paths from typescript but should always return
* POSIX normalized paths for output paths.
*/
export function createSrcToOutPathMapper(
outDir: string | undefined, sampleSrcFileName: string | undefined,
Expand All @@ -986,7 +984,6 @@ export function createSrcToOutPathMapper(
resolve: typeof path.resolve,
relative: typeof path.relative
} = path): (srcFileName: string) => string {
let srcToOutPath: (srcFileName: string) => string;
if (outDir) {
let path: {} = {}; // Ensure we error if we use `path` instead of `host`.
if (sampleSrcFileName == null || sampleOutFileName == null) {
Expand All @@ -1006,11 +1003,18 @@ export function createSrcToOutPathMapper(
srcDirParts[srcDirParts.length - 1 - i] === outDirParts[outDirParts.length - 1 - i])
i++;
const rootDir = srcDirParts.slice(0, srcDirParts.length - i).join('/');
srcToOutPath = (srcFileName) => host.resolve(outDir, host.relative(rootDir, srcFileName));
return (srcFileName) => {
// Note: Before we return the mapped output path, we need to normalize the path delimiters
// because the output path is usually passed to TypeScript which sometimes only expects
// posix normalized paths (e.g. if a custom compiler host is used)
return normalizeSeparators(host.resolve(outDir, host.relative(rootDir, srcFileName)));
};
} else {
srcToOutPath = (srcFileName) => srcFileName;
// Note: Before we return the output path, we need to normalize the path delimiters because
// the output path is usually passed to TypeScript which only passes around posix
// normalized paths (e.g. if a custom compiler host is used)
return (srcFileName) => normalizeSeparators(srcFileName);
}
return srcToOutPath;
}

export function i18nExtract(
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/test/transformers/program_spec.ts
Expand Up @@ -604,13 +604,13 @@ describe('ng program', () => {
it('should work on windows with normalized paths', () => {
const mapper =
createSrcToOutPathMapper('c:/tmp/out', 'c:/tmp/a/x.ts', 'c:/tmp/out/a/x.js', path.win32);
expect(mapper('c:/tmp/b/y.js')).toBe('c:\\tmp\\out\\b\\y.js');
expect(mapper('c:/tmp/b/y.js')).toBe('c:/tmp/out/b/y.js');
});

it('should work on windows with non-normalized paths', () => {
const mapper = createSrcToOutPathMapper(
'c:\\tmp\\out', 'c:\\tmp\\a\\x.ts', 'c:\\tmp\\out\\a\\x.js', path.win32);
expect(mapper('c:\\tmp\\b\\y.js')).toBe('c:\\tmp\\out\\b\\y.js');
expect(mapper('c:\\tmp\\b\\y.js')).toBe('c:/tmp/out/b/y.js');
});
});

Expand Down

0 comments on commit 0ada23a

Please sign in to comment.