From 0ada23a5fb9c2e0ebd33301d192febefa309ddb9 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 12 Nov 2018 17:56:51 +0100 Subject: [PATCH] fix(compiler-cli): only pass canonical genfile paths to compiler host (#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 --- .../compiler-cli/src/transformers/program.ts | 20 +++++++++++-------- .../test/transformers/program_spec.ts | 4 ++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/compiler-cli/src/transformers/program.ts b/packages/compiler-cli/src/transformers/program.ts index 79b3ee4f18..8a10d93529 100644 --- a/packages/compiler-cli/src/transformers/program.ts +++ b/packages/compiler-cli/src/transformers/program.ts @@ -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, @@ -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) { @@ -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( diff --git a/packages/compiler-cli/test/transformers/program_spec.ts b/packages/compiler-cli/test/transformers/program_spec.ts index c5afa87a68..951b1ffbd1 100644 --- a/packages/compiler-cli/test/transformers/program_spec.ts +++ b/packages/compiler-cli/test/transformers/program_spec.ts @@ -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'); }); });