From dee95994b8e5def7c7880038d64968da93fc04b4 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 7 Apr 2021 11:25:36 -0400 Subject: [PATCH] refactor(compiler-cli): support `ts.SourceFile` versioning (#41475) Generally, the compiler assumes that `ts.SourceFile`s are immutable objects. If a new `ts.Program` is compared to an old one, and a `ts.SourceFile` within that program has not changed its object identity, the compiler will assume that its prior analysis and understanding of that source file is still valid. However, not all TypeScript workflows uphold this assumption. For `ts.Program`s that originate from the `ts.LanguageService`, some source files may be re-parsed or otherwise undergo mutations without changing their object identity. This breaks the compiler's incremental workflow. Within such environments, it's necessary to track source file changes differently. In addition to object identity, it's necessary to compare a "version" string associated with each source file, between when that file is analyzed originally and when a new program is presented that still contains it. It's possible for the object identity of the source file to be the same, but the version string to have changed, indicating that the source file should be treated as changed. This commit adds an optional method `getSourceFileVersion` to the `ProgramDriver`, to provide access to version information if available. When this method is present, the compiler will build a map of source file version strings, and use this map to augment identity comparison during incremental compilation. PR Close #41475 --- .../src/ngtsc/core/src/compiler.ts | 29 +++++++++--- .../src/ngtsc/incremental/src/incremental.ts | 45 ++++++++++++++----- .../src/ngtsc/incremental/src/state.ts | 5 +++ .../src/ngtsc/incremental/test/BUILD.bazel | 29 ++++++++++++ .../incremental/test/incremental_spec.ts | 41 +++++++++++++++++ .../src/ngtsc/program_driver/src/api.ts | 11 +++++ 6 files changed, 144 insertions(+), 16 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/incremental/test/BUILD.bazel create mode 100644 packages/compiler-cli/src/ngtsc/incremental/test/incremental_spec.ts diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 1c312c5fd1..90352bf11c 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -13,7 +13,7 @@ import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecorato import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../cycles'; import {COMPILER_ERRORS_WITH_GUIDES, ERROR_DETAILS_PAGE_BASE_URL, ErrorCode, ngErrorCode} from '../../diagnostics'; import {checkForPrivateExports, ReferenceGraph} from '../../entry_point'; -import {AbsoluteFsPath, LogicalFileSystem, resolve} from '../../file_system'; +import {absoluteFromSourceFile, AbsoluteFsPath, LogicalFileSystem, resolve} from '../../file_system'; import {AbsoluteModuleStrategy, AliasingHost, AliasStrategy, DefaultImportTracker, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, PrivateExportAliasingHost, R3SymbolsImportRewriter, Reference, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesAliasingHost, UnifiedModulesStrategy} from '../../imports'; import {IncrementalBuildStrategy, IncrementalCompilation, IncrementalState} from '../../incremental'; import {SemanticSymbol} from '../../incremental/semantic_graph'; @@ -32,7 +32,7 @@ import {ivySwitchTransform} from '../../switch'; import {aliasTransformFactory, CompilationMode, declarationTransformFactory, DecoratorHandler, DtsTransformRegistry, ivyTransformFactory, TraitCompiler} from '../../transform'; import {TemplateTypeCheckerImpl} from '../../typecheck'; import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig} from '../../typecheck/api'; -import {getSourceFileOrNull, isDtsPath, resolveModuleName} from '../../util/src/typescript'; +import {getSourceFileOrNull, isDtsPath, resolveModuleName, toUnredirectedSourceFile} from '../../util/src/typescript'; import {LazyRoute, NgCompilerAdapter, NgCompilerOptions} from '../api'; import {compileUndecoratedClassesWithAngularFeatures} from './config'; @@ -159,7 +159,8 @@ export function incrementalFromCompilerTicket( } const incrementalCompilation = IncrementalCompilation.incremental( - newProgram, oldProgram, oldState, modifiedResourceFiles, perfRecorder); + newProgram, versionMapFromProgram(newProgram, programDriver), oldProgram, oldState, + modifiedResourceFiles, perfRecorder); return { kind: CompilationTicketKind.IncrementalTypeScript, @@ -188,7 +189,8 @@ export function incrementalFromStateTicket( perfRecorder = ActivePerfRecorder.zeroedToNow(); } const incrementalCompilation = IncrementalCompilation.incremental( - newProgram, oldProgram, oldState, modifiedResourceFiles, perfRecorder); + newProgram, versionMapFromProgram(newProgram, programDriver), oldProgram, oldState, + modifiedResourceFiles, perfRecorder); return { kind: CompilationTicketKind.IncrementalTypeScript, newProgram, @@ -282,7 +284,8 @@ export class NgCompiler { ticket.tsProgram, ticket.programDriver, ticket.incrementalBuildStrategy, - IncrementalCompilation.fresh(ticket.tsProgram), + IncrementalCompilation.fresh( + ticket.tsProgram, versionMapFromProgram(ticket.tsProgram, ticket.programDriver)), ticket.enableTemplateTypeChecker, ticket.usePoisonedData, ticket.perfRecorder, @@ -1175,4 +1178,20 @@ class NotifyingProgramDriverWrapper implements ProgramDriver { this.delegate.updateFiles(contents, updateMode); this.notifyNewProgram(this.delegate.getProgram()); } + + getSourceFileVersion = this.delegate.getSourceFileVersion?.bind(this); } + +function versionMapFromProgram( + program: ts.Program, driver: ProgramDriver): Map|null { + if (driver.getSourceFileVersion === undefined) { + return null; + } + + const versions = new Map(); + for (const possiblyRedirectedSourceFile of program.getSourceFiles()) { + const sf = toUnredirectedSourceFile(possiblyRedirectedSourceFile); + versions.set(absoluteFromSourceFile(sf), driver.getSourceFileVersion(sf)); + } + return versions; +} \ No newline at end of file diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/incremental.ts b/packages/compiler-cli/src/ngtsc/incremental/src/incremental.ts index 42fc222137..e0ef06c463 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/src/incremental.ts +++ b/packages/compiler-cli/src/ngtsc/incremental/src/incremental.ts @@ -76,7 +76,7 @@ export class IncrementalCompilation implements IncrementalBuild|null, private step: IncrementalStep|null) { this._state = state; // The compilation begins in analysis phase. @@ -90,26 +90,29 @@ export class IncrementalCompilation implements IncrementalBuild|null): + IncrementalCompilation { const state: IncrementalState = { kind: IncrementalStateKind.Fresh, }; - return new IncrementalCompilation(state, new FileDependencyGraph(), /* reuse */ null); + return new IncrementalCompilation(state, new FileDependencyGraph(), versions, /* reuse */ null); } static incremental( - program: ts.Program, oldProgram: ts.Program, oldState: IncrementalState, - modifiedResourceFiles: Set|null, perf: PerfRecorder): IncrementalCompilation { + program: ts.Program, newVersions: Map|null, oldProgram: ts.Program, + oldState: IncrementalState, modifiedResourceFiles: Set|null, + perf: PerfRecorder): IncrementalCompilation { return perf.inPhase(PerfPhase.Reconciliation, () => { - let priorAnalysis: AnalyzedIncrementalState; const physicallyChangedTsFiles = new Set(); const changedResourceFiles = new Set(modifiedResourceFiles ?? []); + + let priorAnalysis: AnalyzedIncrementalState; switch (oldState.kind) { case IncrementalStateKind.Fresh: // Since this line of program has never been successfully analyzed to begin with, treat // this as a fresh compilation. - return IncrementalCompilation.fresh(program); + return IncrementalCompilation.fresh(program, newVersions); case IncrementalStateKind.Analyzed: // The most recent program was analyzed successfully, so we can use that as our prior // state and don't need to consider any other deltas except changes in the most recent @@ -129,6 +132,8 @@ export class IncrementalCompilation implements IncrementalBuild toUnredirectedSourceFile(sf)); const oldFiles = new Set(oldFilesArray); const deletedTsFiles = new Set(oldFilesArray.map(sf => absoluteFromSourceFile(sf))); @@ -141,14 +146,31 @@ export class IncrementalCompilation implements IncrementalBuild; + + /** + * Map of source file paths to the version of this file as seen in the compilation. + */ + versions: Map|null; } /** diff --git a/packages/compiler-cli/src/ngtsc/incremental/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/incremental/test/BUILD.bazel new file mode 100644 index 0000000000..333aff0605 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/incremental/test/BUILD.bazel @@ -0,0 +1,29 @@ +load("//tools:defaults.bzl", "jasmine_node_test", "ts_library") + +package(default_visibility = ["//visibility:public"]) + +ts_library( + name = "test_lib", + testonly = True, + srcs = glob([ + "**/*.ts", + ]), + deps = [ + "//packages:types", + "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/file_system", + "//packages/compiler-cli/src/ngtsc/file_system/testing", + "//packages/compiler-cli/src/ngtsc/incremental", + "//packages/compiler-cli/src/ngtsc/perf", + "//packages/compiler-cli/src/ngtsc/testing", + "@npm//typescript", + ], +) + +jasmine_node_test( + name = "test", + bootstrap = ["//tools/testing:node_no_angular_es5"], + deps = [ + ":test_lib", + ], +) diff --git a/packages/compiler-cli/src/ngtsc/incremental/test/incremental_spec.ts b/packages/compiler-cli/src/ngtsc/incremental/test/incremental_spec.ts new file mode 100644 index 0000000000..cd2b92676f --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/incremental/test/incremental_spec.ts @@ -0,0 +1,41 @@ +/** + * @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 {absoluteFrom, getSourceFileOrError} from '../../file_system'; +import {runInEachFileSystem} from '../../file_system/testing'; +import {NOOP_PERF_RECORDER} from '../../perf'; +import {makeProgram} from '../../testing'; +import {IncrementalCompilation} from '../src/incremental'; + +runInEachFileSystem(() => { + describe('incremental reconciliation', () => { + it('should treat source files with changed versions as changed', () => { + const FOO_PATH = absoluteFrom('/foo.ts'); + const {program} = makeProgram([ + {name: FOO_PATH, contents: `export const FOO = true;`}, + ]); + const fooSf = getSourceFileOrError(program, FOO_PATH); + + const versionMapFirst = new Map([[FOO_PATH, 'version.1']]); + const firstCompilation = IncrementalCompilation.fresh( + program, + versionMapFirst, + ); + firstCompilation.recordSuccessfulAnalysis(null!); + firstCompilation.recordSuccessfulEmit(fooSf); + + const versionMapSecond = new Map([[FOO_PATH, 'version.2']]); + const secondCompilation = IncrementalCompilation.incremental( + program, versionMapSecond, program, firstCompilation.state, new Set(), + NOOP_PERF_RECORDER); + + secondCompilation.recordSuccessfulAnalysis(null!); + expect(secondCompilation.safeToSkipEmit(fooSf)).toBeFalse(); + }); + }); +}); \ No newline at end of file diff --git a/packages/compiler-cli/src/ngtsc/program_driver/src/api.ts b/packages/compiler-cli/src/ngtsc/program_driver/src/api.ts index d8008489a8..a2a0989bd3 100644 --- a/packages/compiler-cli/src/ngtsc/program_driver/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/program_driver/src/api.ts @@ -26,6 +26,17 @@ export interface ProgramDriver { * included in the type-checking program. */ updateFiles(contents: Map, updateMode: UpdateMode): void; + + /** + * Retrieve a string version for a given `ts.SourceFile`, which much change when the contents of + * the file have changed. + * + * If this method is present, the compiler will use these versions in addition to object identity + * for `ts.SourceFile`s to determine what's changed between two incremental programs. This is + * valuable for some clients (such as the Language Service) that treat `ts.SourceFile`s as mutable + * objects. + */ + getSourceFileVersion?(sf: ts.SourceFile): string; } export enum UpdateMode {