From 3166cffd28e12d882bef3e55472e0a50f4cfc6f5 Mon Sep 17 00:00:00 2001 From: Jon Wallsten Date: Mon, 8 Jul 2019 16:50:19 +0200 Subject: [PATCH] fix(compiler-cli): Return original sourceFile instead of redirected sourceFile from getSourceFile (#26036) Closes #22524 PR Close #26036 --- .../src/ngtsc/typecheck/src/host.ts | 7 ++++ .../src/transformers/compiler_host.ts | 6 +++ .../test/ngtsc/incremental_spec.ts | 31 ++++++++++++++ .../compiler-cli/test/perform_watch_spec.ts | 41 +++++++++++++++++++ 4 files changed, 85 insertions(+) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts index 70e339b7ca..d27f587127 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts @@ -39,6 +39,13 @@ export class TypeCheckProgramHost implements ts.CompilerHost { sf = this.delegate.getSourceFile( fileName, languageVersion, onError, shouldCreateNewSourceFile); sf && this.sfMap.set(fileName, sf); + } else { + // TypeScript doesn't allow returning redirect source files. To avoid unforseen errors we + // return the original source file instead of the redirect target. + const redirectInfo = (sf as any).redirectInfo; + if (redirectInfo !== undefined) { + sf = redirectInfo.unredirected; + } } return sf; } diff --git a/packages/compiler-cli/src/transformers/compiler_host.ts b/packages/compiler-cli/src/transformers/compiler_host.ts index 517b974ec7..739cd38e00 100644 --- a/packages/compiler-cli/src/transformers/compiler_host.ts +++ b/packages/compiler-cli/src/transformers/compiler_host.ts @@ -441,6 +441,12 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos fileName, summary.text, this.options.target || ts.ScriptTarget.Latest); } sf = summary.sourceFile; + // TypeScript doesn't allow returning redirect source files. To avoid unforseen errors we + // return the original source file instead of the redirect target. + const redirectInfo = (sf as any).redirectInfo; + if (redirectInfo !== undefined) { + sf = redirectInfo.unredirected; + } genFileNames = []; } } diff --git a/packages/compiler-cli/test/ngtsc/incremental_spec.ts b/packages/compiler-cli/test/ngtsc/incremental_spec.ts index 9cf9198feb..ee288769d3 100644 --- a/packages/compiler-cli/test/ngtsc/incremental_spec.ts +++ b/packages/compiler-cli/test/ngtsc/incremental_spec.ts @@ -202,6 +202,37 @@ runInEachFileSystem(() => { // If program reuse were configured incorrectly (as was responsible for // https://github.com/angular/angular/issues/30079), this would have crashed. }); + + // https://github.com/angular/angular/pull/26036 + it('should handle redirected source files', () => { + env.tsconfig({fullTemplateTypeCheck: true}); + + // This file structure has an identical version of "a" under the root node_modules and inside + // of "b". Because their package.json file indicates it is the exact same version of "a", + // TypeScript will transform the source file of "node_modules/b/node_modules/a/index.d.ts" + // into a redirect to "node_modules/a/index.d.ts". During incremental compilations, we must + // assure not to reintroduce "node_modules/b/node_modules/a/index.d.ts" as its redirected + // source file, but instead use its original file. + env.write('node_modules/a/index.js', `export class ServiceA {}`); + env.write('node_modules/a/index.d.ts', `export declare class ServiceA {}`); + env.write('node_modules/a/package.json', `{"name": "a", "version": "1.0"}`); + env.write('node_modules/b/node_modules/a/index.js', `export class ServiceA {}`); + env.write('node_modules/b/node_modules/a/index.d.ts', `export declare class ServiceA {}`); + env.write('node_modules/b/node_modules/a/package.json', `{"name": "a", "version": "1.0"}`); + env.write('node_modules/b/index.js', `export {ServiceA as ServiceB} from 'a';`); + env.write('node_modules/b/index.d.ts', `export {ServiceA as ServiceB} from 'a';`); + env.write('test.ts', ` + import {ServiceA} from 'a'; + import {ServiceB} from 'b'; + `); + env.driveMain(); + env.flushWrittenFileTracking(); + + // Pretend a change was made to test.ts. If redirect sources were introduced into the new + // program, this would fail due to an assertion failure in TS. + env.invalidateCachedFile('test.ts'); + env.driveMain(); + }); }); function setupFooBarProgram(env: NgtscTestEnvironment) { diff --git a/packages/compiler-cli/test/perform_watch_spec.ts b/packages/compiler-cli/test/perform_watch_spec.ts index d40531418c..b38d732ebf 100644 --- a/packages/compiler-cli/test/perform_watch_spec.ts +++ b/packages/compiler-cli/test/perform_watch_spec.ts @@ -107,6 +107,47 @@ describe('perform watch', () => { expect(getSourceFileSpy !).toHaveBeenCalledWith(utilTsPath, ts.ScriptTarget.ES5); }); + // https://github.com/angular/angular/pull/26036 + it('should handle redirected source files', () => { + const config = createConfig(); + const host = new MockWatchHost(config); + host.createCompilerHost = (options: ng.CompilerOptions) => { + const ngHost = ng.createCompilerHost({options}); + return ngHost; + }; + + // This file structure has an identical version of "a" under the root node_modules and inside + // of "b". Because their package.json file indicates it is the exact same version of "a", + // TypeScript will transform the source file of "node_modules/b/node_modules/a/index.d.ts" + // into a redirect to "node_modules/a/index.d.ts". During watch compilations, we must assure + // not to reintroduce "node_modules/b/node_modules/a/index.d.ts" as its redirected source file, + // but instead using its original file. + testSupport.writeFiles({ + 'node_modules/a/index.js': `export class ServiceA {}`, + 'node_modules/a/index.d.ts': `export declare class ServiceA {}`, + 'node_modules/a/package.json': `{"name": "a", "version": "1.0"}`, + 'node_modules/b/node_modules/a/index.js': `export class ServiceA {}`, + 'node_modules/b/node_modules/a/index.d.ts': `export declare class ServiceA {}`, + 'node_modules/b/node_modules/a/package.json': `{"name": "a", "version": "1.0"}`, + 'node_modules/b/index.js': `export {ServiceA as ServiceB} from 'a';`, + 'node_modules/b/index.d.ts': `export {ServiceA as ServiceB} from 'a';`, + 'src/index.ts': ` + import {ServiceA} from 'a'; + import {ServiceB} from 'b'; + `, + }); + + const indexTsPath = path.posix.join(testSupport.basePath, 'src', 'index.ts'); + + performWatchCompilation(host); + + // Trigger a file change. This recreates the program from the old program. If redirect sources + // were introduced into the new program, this would fail due to an assertion failure in TS. + host.triggerFileChange(FileChangeEvent.Change, indexTsPath); + expectNoDiagnostics(config.options, host.diagnostics); + }); + + it('should recover from static analysis errors', () => { const config = createConfig(); const host = new MockWatchHost(config);