From ffea31f433c1f71b78a2245d52d2969503db1784 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sun, 4 Apr 2021 22:09:17 +0200 Subject: [PATCH] perf(compiler-cli): allow incremental compilation in the presence of redirected source files (#41448) When multiple occurrences of the same package exist within a single TypeScript compilation unit, TypeScript deduplicates the source files by introducing redirected source file proxies. Such proxies are recreated during an incremental compilation even if the original declaration file did not change, which caused the compiler not to reuse any work from the prior compilation. This commit changes the incremental driver to recognize a redirected source file and treat them as their unredirected source file. PR Close #41448 --- .../src/ngtsc/incremental/src/state.ts | 7 ++- .../src/ts_create_program_driver.ts | 17 ++---- .../src/ngtsc/util/src/typescript.ts | 20 +++++++ .../test/ngtsc/incremental_spec.ts | 52 +++++++++++++++++++ 4 files changed, 80 insertions(+), 16 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts index cd5f5fa431..6dcf4706be 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts +++ b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts @@ -13,6 +13,7 @@ import {PerfEvent, PerfPhase, PerfRecorder} from '../../perf'; import {ClassDeclaration} from '../../reflection'; import {ClassRecord, TraitCompiler} from '../../transform'; import {FileTypeCheckingData} from '../../typecheck/src/checker'; +import {toUnredirectedSourceFile} from '../../util/src/typescript'; import {IncrementalBuild} from '../api'; import {SemanticDepGraph, SemanticDepGraphUpdater} from '../semantic_graph'; @@ -85,12 +86,14 @@ export class IncrementalDriver implements IncrementalBuild(oldProgram.getSourceFiles()); + const oldFiles = + new Set(oldProgram.getSourceFiles().map(toUnredirectedSourceFile)); // Assume all the old files were deleted to begin with. Only TS files are tracked. const deletedTsPaths = new Set(tsOnlyFiles(oldProgram).map(sf => sf.fileName)); - for (const newFile of newProgram.getSourceFiles()) { + for (const possiblyRedirectedNewFile of newProgram.getSourceFiles()) { + const newFile = toUnredirectedSourceFile(possiblyRedirectedNewFile); if (!newFile.isDeclarationFile) { // This file exists in the new program, so remove it from `deletedTsPaths`. deletedTsPaths.delete(newFile.fileName); diff --git a/packages/compiler-cli/src/ngtsc/program_driver/src/ts_create_program_driver.ts b/packages/compiler-cli/src/ngtsc/program_driver/src/ts_create_program_driver.ts index 74acda952e..4fce91fa2d 100644 --- a/packages/compiler-cli/src/ngtsc/program_driver/src/ts_create_program_driver.ts +++ b/packages/compiler-cli/src/ngtsc/program_driver/src/ts_create_program_driver.ts @@ -8,17 +8,9 @@ import * as ts from 'typescript'; -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - import {AbsoluteFsPath} from '../../file_system'; import {copyFileShimData, retagAllTsFiles, ShimReferenceTagger, untagAllTsFiles} from '../../shims'; -import {RequiredDelegations} from '../../util/src/typescript'; +import {RequiredDelegations, toUnredirectedSourceFile} from '../../util/src/typescript'; import {ProgramDriver, UpdateMode} from './api'; @@ -114,12 +106,9 @@ class UpdatedProgramHost extends DelegatingCompilerHost { } else { sf = delegateSf; } - // TypeScript doesn't allow returning redirect source files. To avoid unforseen errors we + // TypeScript doesn't allow returning redirect source files. To avoid unforeseen errors we // return the original source file instead of the redirect target. - const redirectInfo = (sf as any).redirectInfo; - if (redirectInfo !== undefined) { - sf = redirectInfo.unredirected; - } + sf = toUnredirectedSourceFile(sf); this.shimTagger.tag(sf); return sf; diff --git a/packages/compiler-cli/src/ngtsc/util/src/typescript.ts b/packages/compiler-cli/src/ngtsc/util/src/typescript.ts index 6063442aad..565fab3026 100644 --- a/packages/compiler-cli/src/ngtsc/util/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/util/src/typescript.ts @@ -161,3 +161,23 @@ export type SubsetOfKeys = K; export type RequiredDelegations = { [M in keyof Required]: T[M]; }; + +/** + * Source files may become redirects to other source files when their package name and version are + * identical. TypeScript creates a proxy source file for such source files which has an internal + * `redirectInfo` property that refers to the original source file. + */ +interface RedirectedSourceFile extends ts.SourceFile { + redirectInfo?: {unredirected: ts.SourceFile;}; +} + +/** + * Obtains the non-redirected source file for `sf`. + */ +export function toUnredirectedSourceFile(sf: ts.SourceFile): ts.SourceFile { + const redirectInfo = (sf as RedirectedSourceFile).redirectInfo; + if (redirectInfo === undefined) { + return sf; + } + return redirectInfo.unredirected; +} diff --git a/packages/compiler-cli/test/ngtsc/incremental_spec.ts b/packages/compiler-cli/test/ngtsc/incremental_spec.ts index 5b94f8ccc8..9f6c4f56e1 100644 --- a/packages/compiler-cli/test/ngtsc/incremental_spec.ts +++ b/packages/compiler-cli/test/ngtsc/incremental_spec.ts @@ -524,6 +524,58 @@ runInEachFileSystem(() => { env.driveMain(); }); + it('should allow incremental compilation with 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, the + // redirected "node_modules/b/node_modules/a/index.d.ts" source file should be considered as + // its unredirected source file to avoid a change in declaration files. + 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('component1.ts', ` + import {Component} from '@angular/core'; + import {ServiceA} from 'a'; + import {ServiceB} from 'b'; + + @Component({selector: 'cmp', template: 'cmp'}) + export class Cmp1 {} + `); + env.write('component2.ts', ` + import {Component} from '@angular/core'; + import {ServiceA} from 'a'; + import {ServiceB} from 'b'; + + @Component({selector: 'cmp2', template: 'cmp'}) + export class Cmp2 {} + `); + env.driveMain(); + env.flushWrittenFileTracking(); + + // Now update `component1.ts` and change its imports to avoid complete structure reuse, which + // forces recreation of source file redirects. + env.write('component1.ts', ` + import {Component} from '@angular/core'; + import {ServiceA} from 'a'; + + @Component({selector: 'cmp', template: 'cmp'}) + export class Cmp1 {} + `); + env.driveMain(); + + const written = env.getFilesWrittenSinceLastFlush(); + expect(written).toContain('/component1.js'); + expect(written).not.toContain('/component2.js'); + }); + describe('template type-checking', () => { beforeEach(() => { env.tsconfig({strictTemplates: true});