diff --git a/packages/compiler-cli/BUILD.bazel b/packages/compiler-cli/BUILD.bazel index 773af85c62..cb3c783dd7 100644 --- a/packages/compiler-cli/BUILD.bazel +++ b/packages/compiler-cli/BUILD.bazel @@ -31,6 +31,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/indexer", "//packages/compiler-cli/src/ngtsc/perf", "//packages/compiler-cli/src/ngtsc/reflection", + "//packages/compiler-cli/src/ngtsc/shims", "//packages/compiler-cli/src/ngtsc/typecheck", "@npm//@bazel/typescript", "@npm//@types/node", diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index d1e3d33885..bf81c77638 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -17,6 +17,7 @@ import {NgCompilerOptions} from './core/api'; import {TrackedIncrementalBuildStrategy} from './incremental'; import {IndexedComponent} from './indexer'; import {NOOP_PERF_RECORDER, PerfRecorder, PerfTracker} from './perf'; +import {retagAllTsFiles, untagAllTsFiles} from './shims'; import {ReusedProgramStrategy} from './typecheck'; @@ -68,14 +69,26 @@ export class NgtscProgram implements api.Program { } this.closureCompilerEnabled = !!options.annotateForClosureCompiler; - const reuseProgram = oldProgram && oldProgram.reuseTsProgram; + const reuseProgram = oldProgram?.reuseTsProgram; this.host = NgCompilerHost.wrap(delegateHost, rootNames, options, reuseProgram ?? null); + if (reuseProgram !== undefined) { + // Prior to reusing the old program, restore shim tagging for all its `ts.SourceFile`s. + // TypeScript checks the `referencedFiles` of `ts.SourceFile`s for changes when evaluating + // incremental reuse of data from the old program, so it's important that these match in order + // to get the most benefit out of reuse. + retagAllTsFiles(reuseProgram); + } + this.tsProgram = ts.createProgram(this.host.inputFiles, options, this.host, reuseProgram); this.reuseTsProgram = this.tsProgram; this.host.postProgramCreationCleanup(); + // Shim tagging has served its purpose, and tags can now be removed from all `ts.SourceFile`s in + // the program. + untagAllTsFiles(this.tsProgram); + const reusedProgramStrategy = new ReusedProgramStrategy( this.tsProgram, this.host, this.options, this.host.shimExtensionPrefixes); @@ -93,6 +106,10 @@ export class NgtscProgram implements api.Program { return this.tsProgram; } + getReuseTsProgram(): ts.Program { + return this.reuseTsProgram; + } + getTsOptionDiagnostics(cancellationToken?: ts.CancellationToken| undefined): readonly ts.Diagnostic[] { return this.tsProgram.getOptionsDiagnostics(cancellationToken); @@ -248,6 +265,7 @@ export class NgtscProgram implements api.Program { })); this.perfRecorder.stop(fileEmitSpan); } + this.perfRecorder.stop(emitSpan); if (this.perfTracker !== null && this.options.tracePerformance !== undefined) { diff --git a/packages/compiler-cli/src/ngtsc/shims/index.ts b/packages/compiler-cli/src/ngtsc/shims/index.ts index d897845c4e..9cc388f5b7 100644 --- a/packages/compiler-cli/src/ngtsc/shims/index.ts +++ b/packages/compiler-cli/src/ngtsc/shims/index.ts @@ -9,7 +9,7 @@ /// export {ShimAdapter} from './src/adapter'; -export {copyFileShimData, isShim} from './src/expando'; +export {copyFileShimData, isShim, retagAllTsFiles, retagTsFile, sfExtensionData, untagAllTsFiles, untagTsFile} from './src/expando'; export {FactoryGenerator, generatedFactoryTransform} from './src/factory_generator'; export {ShimReferenceTagger} from './src/reference_tagger'; export {SummaryGenerator} from './src/summary_generator'; diff --git a/packages/compiler-cli/src/ngtsc/shims/src/adapter.ts b/packages/compiler-cli/src/ngtsc/shims/src/adapter.ts index f07d8b6ea6..ab4494568d 100644 --- a/packages/compiler-cli/src/ngtsc/shims/src/adapter.ts +++ b/packages/compiler-cli/src/ngtsc/shims/src/adapter.ts @@ -12,7 +12,7 @@ import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath} from '../../file_s import {isDtsPath} from '../../util/src/typescript'; import {PerFileShimGenerator, TopLevelShimGenerator} from '../api'; -import {isFileShimSourceFile, isShim, NgExtension, sfExtensionData} from './expando'; +import {isFileShimSourceFile, isShim, sfExtensionData} from './expando'; import {makeShimFileName} from './util'; interface ShimGeneratorData { diff --git a/packages/compiler-cli/src/ngtsc/shims/src/expando.ts b/packages/compiler-cli/src/ngtsc/shims/src/expando.ts index 71b3c81e6f..3d03ce42ba 100644 --- a/packages/compiler-cli/src/ngtsc/shims/src/expando.ts +++ b/packages/compiler-cli/src/ngtsc/shims/src/expando.ts @@ -21,7 +21,16 @@ export const NgExtension = Symbol('NgExtension'); export interface NgExtensionData { isTopLevelShim: boolean; fileShim: NgFileShimData|null; + + /** + * The contents of the `referencedFiles` array, before modification by a `ShimReferenceTagger`. + */ originalReferencedFiles: ReadonlyArray|null; + + /** + * The contents of the `referencedFiles` array, after modification by a `ShimReferenceTagger`. + */ + taggedReferenceFiles: ReadonlyArray|null; } /** @@ -65,6 +74,7 @@ export function sfExtensionData(sf: ts.SourceFile): NgExtensionData { isTopLevelShim: false, fileShim: null, originalReferencedFiles: null, + taggedReferenceFiles: null, }; extSf[NgExtension] = extension; return extension; @@ -110,3 +120,53 @@ export function copyFileShimData(from: ts.SourceFile, to: ts.SourceFile): void { } sfExtensionData(to).fileShim = sfExtensionData(from).fileShim; } + +/** + * For those `ts.SourceFile`s in the `program` which have previously been tagged by a + * `ShimReferenceTagger`, restore the original `referencedFiles` array that does not have shim tags. + */ +export function untagAllTsFiles(program: ts.Program): void { + for (const sf of program.getSourceFiles()) { + untagTsFile(sf); + } +} + +/** + * For those `ts.SourceFile`s in the `program` which have previously been tagged by a + * `ShimReferenceTagger`, re-apply the effects of tagging by updating the `referencedFiles` array to + * the tagged version produced previously. + */ +export function retagAllTsFiles(program: ts.Program): void { + for (const sf of program.getSourceFiles()) { + retagTsFile(sf); + } +} + +/** + * Restore the original `referencedFiles` for the given `ts.SourceFile`. + */ +export function untagTsFile(sf: ts.SourceFile): void { + if (sf.isDeclarationFile || !isExtended(sf)) { + return; + } + + const ext = sfExtensionData(sf); + if (ext.originalReferencedFiles !== null) { + sf.referencedFiles = ext.originalReferencedFiles as Array; + } +} + +/** + * Apply the previously tagged `referencedFiles` to the given `ts.SourceFile`, if it was previously + * tagged. + */ +export function retagTsFile(sf: ts.SourceFile): void { + if (sf.isDeclarationFile || !isExtended(sf)) { + return; + } + + const ext = sfExtensionData(sf); + if (ext.taggedReferenceFiles !== null) { + sf.referencedFiles = ext.taggedReferenceFiles as Array; + } +} diff --git a/packages/compiler-cli/src/ngtsc/shims/src/reference_tagger.ts b/packages/compiler-cli/src/ngtsc/shims/src/reference_tagger.ts index 17899251d4..3e0a3fe428 100644 --- a/packages/compiler-cli/src/ngtsc/shims/src/reference_tagger.ts +++ b/packages/compiler-cli/src/ngtsc/shims/src/reference_tagger.ts @@ -8,10 +8,10 @@ import * as ts from 'typescript'; -import {absoluteFrom, absoluteFromSourceFile} from '../../file_system'; +import {absoluteFromSourceFile} from '../../file_system'; import {isNonDeclarationTsPath} from '../../util/src/typescript'; -import {isExtended as isExtendedSf, isShim, NgExtension, sfExtensionData} from './expando'; +import {isShim, sfExtensionData} from './expando'; import {makeShimFileName} from './util'; /** @@ -48,8 +48,16 @@ export class ShimReferenceTagger { return; } - sfExtensionData(sf).originalReferencedFiles = sf.referencedFiles; - const referencedFiles = [...sf.referencedFiles]; + const ext = sfExtensionData(sf); + + // If this file has never been tagged before, capture its `referencedFiles` in the extension + // data. + if (ext.originalReferencedFiles === null) { + ext.originalReferencedFiles = sf.referencedFiles; + } + + const referencedFiles = [...ext.originalReferencedFiles]; + const sfPath = absoluteFromSourceFile(sf); for (const suffix of this.suffixes) { @@ -60,26 +68,16 @@ export class ShimReferenceTagger { }); } + ext.taggedReferenceFiles = referencedFiles; sf.referencedFiles = referencedFiles; this.tagged.add(sf); } /** - * Restore the original `referencedFiles` values of all tagged `ts.SourceFile`s and disable the - * `ShimReferenceTagger`. + * Disable the `ShimReferenceTagger` and free memory associated with tracking tagged files. */ finalize(): void { this.enabled = false; - for (const sf of this.tagged) { - if (!isExtendedSf(sf)) { - continue; - } - - const extensionData = sfExtensionData(sf); - if (extensionData.originalReferencedFiles !== null) { - sf.referencedFiles = extensionData.originalReferencedFiles! as ts.FileReference[]; - } - } this.tagged.clear(); } } diff --git a/packages/compiler-cli/src/ngtsc/shims/test/reference_tagger_spec.ts b/packages/compiler-cli/src/ngtsc/shims/test/reference_tagger_spec.ts index 6bf2a600b0..807d0996f9 100644 --- a/packages/compiler-cli/src/ngtsc/shims/test/reference_tagger_spec.ts +++ b/packages/compiler-cli/src/ngtsc/shims/test/reference_tagger_spec.ts @@ -12,6 +12,7 @@ import {absoluteFrom as _, AbsoluteFsPath, getSourceFileOrError} from '../../fil import {runInEachFileSystem} from '../../file_system/testing'; import {makeProgram} from '../../testing'; import {ShimAdapter} from '../src/adapter'; +import {retagTsFile, untagTsFile} from '../src/expando'; import {ShimReferenceTagger} from '../src/reference_tagger'; import {TestShimGenerator} from './util'; @@ -67,40 +68,6 @@ runInEachFileSystem(() => { expect(shimSf.referencedFiles).toEqual([]); }); - it('should remove tags during finalization', () => { - const tagger = new ShimReferenceTagger(['test1', 'test2']); - - const fileName = _('/file.ts'); - const sf = makeArbitrarySf(fileName); - - expectReferencedFiles(sf, []); - - tagger.tag(sf); - expectReferencedFiles(sf, ['/file.test1.ts', '/file.test2.ts']); - - tagger.finalize(); - expectReferencedFiles(sf, []); - }); - - it('should not remove references it did not add during finalization', () => { - const tagger = new ShimReferenceTagger(['test1', 'test2']); - const fileName = _('/file.ts'); - const libFileName = _('/lib.d.ts'); - - const sf = makeSf(fileName, ` - /// - export const UNIMPORTANT = true; - `); - - expectReferencedFiles(sf, [libFileName]); - - tagger.tag(sf); - expectReferencedFiles(sf, ['/file.test1.ts', '/file.test2.ts', libFileName]); - - tagger.finalize(); - expectReferencedFiles(sf, [libFileName]); - }); - it('should not tag shims after finalization', () => { const tagger = new ShimReferenceTagger(['test1', 'test2']); tagger.finalize(); @@ -111,6 +78,56 @@ runInEachFileSystem(() => { tagger.tag(sf); expectReferencedFiles(sf, []); }); + + it('should not overwrite original referencedFiles', () => { + const tagger = new ShimReferenceTagger(['test']); + + const fileName = _('/file.ts'); + const sf = makeArbitrarySf(fileName); + sf.referencedFiles = [{ + fileName: _('/other.ts'), + pos: 0, + end: 0, + }]; + + tagger.tag(sf); + expectReferencedFiles(sf, ['/other.ts', '/file.test.ts']); + }); + + it('should always tag against the original referencedFiles', () => { + const tagger1 = new ShimReferenceTagger(['test1']); + const tagger2 = new ShimReferenceTagger(['test2']); + + const fileName = _('/file.ts'); + const sf = makeArbitrarySf(fileName); + + tagger1.tag(sf); + tagger2.tag(sf); + expectReferencedFiles(sf, ['/file.test2.ts']); + }); + + describe('tagging and untagging', () => { + it('should be able to untag references and retag them later', () => { + const tagger = new ShimReferenceTagger(['test']); + + const fileName = _('/file.ts'); + const sf = makeArbitrarySf(fileName); + sf.referencedFiles = [{ + fileName: _('/other.ts'), + pos: 0, + end: 0, + }]; + + tagger.tag(sf); + expectReferencedFiles(sf, ['/other.ts', '/file.test.ts']); + + untagTsFile(sf); + expectReferencedFiles(sf, ['/other.ts']); + + retagTsFile(sf); + expectReferencedFiles(sf, ['/other.ts', '/file.test.ts']); + }); + }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/testing/index.ts b/packages/compiler-cli/src/ngtsc/testing/index.ts index 6e6da32033..354d8a3876 100644 --- a/packages/compiler-cli/src/ngtsc/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/testing/index.ts @@ -5,4 +5,4 @@ * 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 */ -export {getDeclaration, makeProgram} from './src/utils'; +export {expectCompleteReuse, getDeclaration, makeProgram} from './src/utils'; diff --git a/packages/compiler-cli/src/ngtsc/testing/src/utils.ts b/packages/compiler-cli/src/ngtsc/testing/src/utils.ts index 2b56b0a877..f2c84d263d 100644 --- a/packages/compiler-cli/src/ngtsc/testing/src/utils.ts +++ b/packages/compiler-cli/src/ngtsc/testing/src/utils.ts @@ -97,6 +97,23 @@ export function walkForDeclaration(name: string, rootNode: ts.Node): ts.Declarat return chosenDecl; } +const COMPLETE_REUSE_FAILURE_MESSAGE = + 'The original program was not reused completely, even though no changes should have been made to its structure'; + +/** + * Extracted from TypeScript's internal enum `StructureIsReused`. + */ +enum TsStructureIsReused { + Not = 0, + SafeModules = 1, + Completely = 2, +} + +export function expectCompleteReuse(oldProgram: ts.Program): void { + // Assert complete reuse using TypeScript's private API. + expect((oldProgram as any).structureIsReused) + .toBe(TsStructureIsReused.Completely, COMPLETE_REUSE_FAILURE_MESSAGE); +} function bindingNameEquals(node: ts.BindingName, name: string): boolean { if (ts.isIdentifier(node)) { diff --git a/packages/compiler-cli/src/ngtsc/tsc_plugin.ts b/packages/compiler-cli/src/ngtsc/tsc_plugin.ts index 0afc0a168b..7c87be7f22 100644 --- a/packages/compiler-cli/src/ngtsc/tsc_plugin.ts +++ b/packages/compiler-cli/src/ngtsc/tsc_plugin.ts @@ -13,6 +13,7 @@ import {NgCompilerOptions, UnifiedModulesHost} from './core/api'; import {NodeJSFileSystem, setFileSystem} from './file_system'; import {PatchedProgramIncrementalBuildStrategy} from './incremental'; import {NOOP_PERF_RECORDER} from './perf'; +import {untagAllTsFiles} from './shims'; import {ReusedProgramStrategy} from './typecheck/src/augmented_program'; // The following is needed to fix a the chicken-and-egg issue where the sync (into g3) script will @@ -80,6 +81,9 @@ export class NgTscPlugin implements TscPlugin { wrapHost( host: ts.CompilerHost&UnifiedModulesHost, inputFiles: readonly string[], options: ts.CompilerOptions): PluginCompilerHost { + // TODO(alxhub): Eventually the `wrapHost()` API will accept the old `ts.Program` (if one is + // available). When it does, its `ts.SourceFile`s need to be re-tagged to enable proper + // incremental compilation. this.options = {...this.ngOptions, ...options} as NgCompilerOptions; this.host = NgCompilerHost.wrap(host, inputFiles, this.options, /* oldProgram */ null); return this.host; @@ -92,6 +96,8 @@ export class NgTscPlugin implements TscPlugin { if (this.host === null || this.options === null) { throw new Error('Lifecycle error: setupCompilation() before wrapHost().'); } + this.host.postProgramCreationCleanup(); + untagAllTsFiles(program); const typeCheckStrategy = new ReusedProgramStrategy( program, this.host, this.options, this.host.shimExtensionPrefixes); this._compiler = new NgCompiler( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/augmented_program.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/augmented_program.ts index ce17ab66d1..c3c1f73987 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/augmented_program.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/augmented_program.ts @@ -9,6 +9,7 @@ import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../file_system'; +import {retagAllTsFiles, untagAllTsFiles} from '../../shims'; import {TypeCheckingProgramStrategy, UpdateMode} from './api'; import {TypeCheckProgramHost} from './host'; @@ -26,8 +27,10 @@ export class ReusedProgramStrategy implements TypeCheckingProgramStrategy { */ private sfMap = new Map(); + private program: ts.Program = this.originalProgram; + constructor( - private program: ts.Program, private originalHost: ts.CompilerHost, + private originalProgram: ts.Program, private originalHost: ts.CompilerHost, private options: ts.CompilerOptions, private shimExtensionPrefixes: string[]) {} getProgram(): ts.Program { @@ -35,6 +38,17 @@ export class ReusedProgramStrategy implements TypeCheckingProgramStrategy { } updateFiles(contents: Map, updateMode: UpdateMode): void { + if (contents.size === 0) { + // No changes have been requested. Is it safe to skip updating entirely? + // If UpdateMode is Incremental, then yes. If UpdateMode is Complete, then it's safe to skip + // only if there are no active changes already (that would be cleared by the update). + + if (updateMode !== UpdateMode.Complete || this.sfMap.size === 0) { + // No changes would be made to the `ts.Program` anyway, so it's safe to do nothing here. + return; + } + } + if (updateMode === UpdateMode.Complete) { this.sfMap.clear(); } @@ -43,14 +57,25 @@ export class ReusedProgramStrategy implements TypeCheckingProgramStrategy { this.sfMap.set(filePath, ts.createSourceFile(filePath, text, ts.ScriptTarget.Latest, true)); } - const host = - new TypeCheckProgramHost(this.sfMap, this.originalHost, this.shimExtensionPrefixes); + const host = new TypeCheckProgramHost( + this.sfMap, this.originalProgram, this.originalHost, this.shimExtensionPrefixes); + const oldProgram = this.program; + + // Retag the old program's `ts.SourceFile`s with shim tags, to allow TypeScript to reuse the + // most data. + retagAllTsFiles(oldProgram); + this.program = ts.createProgram({ host, rootNames: this.program.getRootFileNames(), options: this.options, - oldProgram: this.program, + oldProgram, }); host.postProgramCreationCleanup(); + + // And untag them afterwards. We explicitly untag both programs here, because the oldProgram + // may still be used for emit and needs to not contain tags. + untagAllTsFiles(this.program); + untagAllTsFiles(oldProgram); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts index 8fb081415a..57a74db930 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/host.ts @@ -35,8 +35,8 @@ export class TypeCheckProgramHost implements ts.CompilerHost { readonly resolveModuleNames?: ts.CompilerHost['resolveModuleNames']; constructor( - sfMap: Map, private delegate: ts.CompilerHost, - private shimExtensionPrefixes: string[]) { + sfMap: Map, private originalProgram: ts.Program, + private delegate: ts.CompilerHost, private shimExtensionPrefixes: string[]) { this.sfMap = sfMap; if (delegate.getDirectories !== undefined) { @@ -52,8 +52,15 @@ export class TypeCheckProgramHost implements ts.CompilerHost { fileName: string, languageVersion: ts.ScriptTarget, onError?: ((message: string) => void)|undefined, shouldCreateNewSourceFile?: boolean|undefined): ts.SourceFile|undefined { - const delegateSf = - this.delegate.getSourceFile(fileName, languageVersion, onError, shouldCreateNewSourceFile)!; + // Try to use the same `ts.SourceFile` as the original program, if possible. This guarantees + // that program reuse will be as efficient as possible. + let delegateSf: ts.SourceFile|undefined = this.originalProgram.getSourceFile(fileName); + if (delegateSf === undefined) { + // Something went wrong and a source file is being requested that's not in the original + // program. Just in case, try to retrieve it from the delegate. + delegateSf = this.delegate.getSourceFile( + fileName, languageVersion, onError, shouldCreateNewSourceFile)!; + } if (delegateSf === undefined) { return undefined; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel index b9974dee6d..ce34bc6aa5 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel @@ -16,6 +16,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/incremental", "//packages/compiler-cli/src/ngtsc/reflection", + "//packages/compiler-cli/src/ngtsc/shims", "//packages/compiler-cli/src/ngtsc/testing", "//packages/compiler-cli/src/ngtsc/typecheck", "//packages/compiler-cli/src/ngtsc/util", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/program_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/program_spec.ts new file mode 100644 index 0000000000..1d14e84ea6 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/program_spec.ts @@ -0,0 +1,98 @@ +/** + * @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 * as ts from 'typescript'; + +import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError} from '../../file_system'; +import {runInEachFileSystem} from '../../file_system/testing'; +import {sfExtensionData, ShimReferenceTagger} from '../../shims'; +import {expectCompleteReuse, makeProgram} from '../../testing'; +import {UpdateMode} from '../src/api'; +import {ReusedProgramStrategy} from '../src/augmented_program'; + +import {createProgramWithNoTemplates} from './test_utils'; + +runInEachFileSystem(() => { + describe('template type-checking program', () => { + it('should not be created if no components need to be checked', () => { + const {program, templateTypeChecker, programStrategy} = createProgramWithNoTemplates(); + templateTypeChecker.refresh(); + // expect() here would create a really long error message, so this is checked manually. + if (programStrategy.getProgram() !== program) { + fail('Template type-checking created a new ts.Program even though it had no changes.'); + } + }); + + it('should have complete reuse if no structural changes are made to shims', () => { + const {program, host, options, typecheckPath} = makeSingleFileProgramWithTypecheckShim(); + const programStrategy = new ReusedProgramStrategy(program, host, options, ['ngtypecheck']); + + // Update /main.ngtypecheck.ts without changing its shape. Verify that the old program was + // reused completely. + programStrategy.updateFiles( + new Map([[typecheckPath, 'export const VERSION = 2;']]), UpdateMode.Complete); + + expectCompleteReuse(program); + }); + + it('should have complete reuse if no structural changes are made to input files', () => { + const {program, host, options, mainPath} = makeSingleFileProgramWithTypecheckShim(); + const programStrategy = new ReusedProgramStrategy(program, host, options, ['ngtypecheck']); + + // Update /main.ts without changing its shape. Verify that the old program was reused + // completely. + programStrategy.updateFiles( + new Map([[mainPath, 'export const STILL_NOT_A_COMPONENT = true;']]), UpdateMode.Complete); + + expectCompleteReuse(program); + }); + }); +}); + +function makeSingleFileProgramWithTypecheckShim(): { + program: ts.Program, + host: ts.CompilerHost, + options: ts.CompilerOptions, + mainPath: AbsoluteFsPath, + typecheckPath: AbsoluteFsPath, +} { + const mainPath = absoluteFrom('/main.ts'); + const typecheckPath = absoluteFrom('/main.ngtypecheck.ts'); + const {program, host, options} = makeProgram([ + { + name: mainPath, + contents: 'export const NOT_A_COMPONENT = true;', + }, + { + name: typecheckPath, + contents: 'export const VERSION = 1;', + } + ]); + + const sf = getSourceFileOrError(program, mainPath); + const typecheckSf = getSourceFileOrError(program, typecheckPath); + + // To ensure this test is validating the correct behavior, the initial conditions of the + // input program must be such that: + // + // 1) /main.ts was previously tagged with a reference to its ngtypecheck shim. + // 2) /main.ngtypecheck.ts is marked as a shim itself. + + // Condition 1: + const tagger = new ShimReferenceTagger(['ngtypecheck']); + tagger.tag(sf); + tagger.finalize(); + + // Condition 2: + sfExtensionData(typecheckSf).fileShim = { + extension: 'ngtypecheck', + generatedFrom: mainPath, + }; + + return {program, host, options, mainPath, typecheckPath}; +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 364c6b9c04..cdc76566f0 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -235,10 +235,18 @@ export function tcb( return res.replace(/\s+/g, ' '); } -export function typecheck( - template: string, source: string, declarations: TestDeclaration[] = [], - additionalSources: {name: AbsoluteFsPath; contents: string}[] = [], - config: Partial = {}, opts: ts.CompilerOptions = {}): ts.Diagnostic[] { +export interface TemplateTestEnvironment { + sf: ts.SourceFile; + program: ts.Program; + templateTypeChecker: TemplateTypeChecker; + programStrategy: ReusedProgramStrategy; +} + +function setupTemplateTypeChecking( + source: string, additionalSources: {name: AbsoluteFsPath; contents: string}[], + config: Partial, opts: ts.CompilerOptions, + makeTypeCheckAdapterFn: (program: ts.Program, sf: ts.SourceFile) => + ProgramTypeCheckAdapter): TemplateTestEnvironment { const typeCheckFilePath = absoluteFrom('/main.ngtypecheck.ts'); const files = [ typescriptLibDts(), @@ -266,48 +274,65 @@ export function typecheck( ]); const fullConfig = {...ALL_ENABLED_CONFIG, ...config}; - const templateUrl = 'synthetic.html'; - const templateFile = new ParseSourceFile(template, templateUrl); - const {nodes, errors} = parseTemplate(template, templateUrl); - if (errors !== undefined) { - throw new Error('Template parse errors: \n' + errors.join('\n')); - } - - const {matcher, pipes} = prepareDeclarations(declarations, decl => { - let declFile = sf; - if (decl.file !== undefined) { - declFile = program.getSourceFile(decl.file)!; - if (declFile === undefined) { - throw new Error(`Unable to locate ${decl.file} for ${decl.type} ${decl.name}`); - } - } - return getClass(declFile, decl.name); - }); - const binder = new R3TargetBinder(matcher); - const boundTarget = binder.bind({template: nodes}); - const clazz = new Reference(getClass(sf, 'TestComponent')); - - const sourceMapping: TemplateSourceMapping = { - type: 'external', - template, - templateUrl, - componentClass: clazz.node, - // Use the class's name for error mappings. - node: clazz.node.name, - }; - - const checkAdapter = createTypeCheckAdapter((ctx: TypeCheckContext) => { - ctx.addTemplate(clazz, boundTarget, pipes, [], sourceMapping, templateFile); - }); - + const checkAdapter = makeTypeCheckAdapterFn(program, sf); const programStrategy = new ReusedProgramStrategy(program, host, options, []); const templateTypeChecker = new TemplateTypeChecker( program, programStrategy, checkAdapter, fullConfig, emitter, reflectionHost, host, NOOP_INCREMENTAL_BUILD); + + return {program, sf, templateTypeChecker, programStrategy}; +} + +export function typecheck( + template: string, source: string, declarations: TestDeclaration[] = [], + additionalSources: {name: AbsoluteFsPath; contents: string}[] = [], + config: Partial = {}, opts: ts.CompilerOptions = {}): ts.Diagnostic[] { + const {sf, templateTypeChecker} = + setupTemplateTypeChecking(source, additionalSources, config, opts, (program, sf) => { + const templateUrl = 'synthetic.html'; + const templateFile = new ParseSourceFile(template, templateUrl); + const {nodes, errors} = parseTemplate(template, templateUrl); + if (errors !== undefined) { + throw new Error('Template parse errors: \n' + errors.join('\n')); + } + + const {matcher, pipes} = prepareDeclarations(declarations, decl => { + let declFile = sf; + if (decl.file !== undefined) { + declFile = program.getSourceFile(decl.file)!; + if (declFile === undefined) { + throw new Error(`Unable to locate ${decl.file} for ${decl.type} ${decl.name}`); + } + } + return getClass(declFile, decl.name); + }); + const binder = new R3TargetBinder(matcher); + const boundTarget = binder.bind({template: nodes}); + const clazz = new Reference(getClass(sf, 'TestComponent')); + + const sourceMapping: TemplateSourceMapping = { + type: 'external', + template, + templateUrl, + componentClass: clazz.node, + // Use the class's name for error mappings. + node: clazz.node.name, + }; + + return createTypeCheckAdapter((ctx: TypeCheckContext) => { + ctx.addTemplate(clazz, boundTarget, pipes, [], sourceMapping, templateFile); + }); + }); + templateTypeChecker.refresh(); return templateTypeChecker.getDiagnosticsForFile(sf); } +export function createProgramWithNoTemplates(): TemplateTestEnvironment { + return setupTemplateTypeChecking( + 'export const NOT_A_COMPONENT = true;', [], {}, {}, () => createTypeCheckAdapter(() => {})); +} + function createTypeCheckAdapter(fn: (ctx: TypeCheckContext) => void): ProgramTypeCheckAdapter { let called = false; return { diff --git a/packages/compiler-cli/test/ngtsc/BUILD.bazel b/packages/compiler-cli/test/ngtsc/BUILD.bazel index 62467c93d9..96e2a67c9e 100644 --- a/packages/compiler-cli/test/ngtsc/BUILD.bazel +++ b/packages/compiler-cli/test/ngtsc/BUILD.bazel @@ -12,6 +12,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/file_system/testing", "//packages/compiler-cli/src/ngtsc/indexer", "//packages/compiler-cli/src/ngtsc/routing", + "//packages/compiler-cli/src/ngtsc/testing", "//packages/compiler-cli/src/ngtsc/util", "//packages/compiler-cli/test:test_utils", "//packages/compiler-cli/test/helpers", diff --git a/packages/compiler-cli/test/ngtsc/env.ts b/packages/compiler-cli/test/ngtsc/env.ts index c32ee50623..281ec58a88 100644 --- a/packages/compiler-cli/test/ngtsc/env.ts +++ b/packages/compiler-cli/test/ngtsc/env.ts @@ -128,6 +128,13 @@ export class NgtscTestEnvironment { return this.oldProgram.getTsProgram(); } + getReuseTsProgram(): ts.Program { + if (this.oldProgram === null) { + throw new Error('No ts.Program has been created yet.'); + } + return (this.oldProgram as NgtscProgram).getReuseTsProgram(); + } + /** * Older versions of the CLI do not provide the `CompilerHost.getModifiedResourceFiles()` method. * This results in the `changedResources` set being `null`. diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 4179b649f0..6d084e5baa 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -9,8 +9,9 @@ import * as ts from 'typescript'; import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics'; -import {absoluteFrom as _, getFileSystem} from '../../src/ngtsc/file_system'; +import {absoluteFrom as _, getFileSystem, getSourceFileOrError} from '../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing'; +import {expectCompleteReuse} from '../../src/ngtsc/testing'; import {loadStandardTestFiles} from '../helpers/src/mock_file_loading'; import {NgtscTestEnvironment} from './env'; @@ -1862,18 +1863,26 @@ export declare class AnimationEvent { expect(env.driveDiagnostics()).toEqual([]); }); - it('should not leave references to shims after execution', () => { - // This test verifies that proper cleanup is performed for the technique being used to - // include shim files in the ts.Program, and that none are left in the referencedFiles of - // any ts.SourceFile after compilation. + it('should not leave referencedFiles in a tagged state', () => { env.enableMultipleCompilations(); env.driveMain(); - for (const sf of env.getTsProgram().getSourceFiles()) { - for (const ref of sf.referencedFiles) { - expect(ref.fileName).not.toContain('.ngtypecheck.ts'); - } - } + const sf = getSourceFileOrError(env.getTsProgram(), _('/test.ts')); + expect(sf.referencedFiles.map(ref => ref.fileName)).toEqual([]); + }); + + it('should allow for complete program reuse during incremental compilations', () => { + env.enableMultipleCompilations(); + + env.write('other.ts', `export const VERSION = 1;`); + + env.driveMain(); + const firstProgram = env.getReuseTsProgram(); + + env.write('other.ts', `export const VERSION = 2;`); + env.driveMain(); + + expectCompleteReuse(firstProgram); }); }); });