fix(language-service): [Ivy] create compiler only when program changes (#39231)

This commit fixes a bug in which a new Ivy Compiler is created every time
language service receives a new request. This is not needed if the
`ts.Program` has not changed.

A new class `CompilerFactory` is created to manage Compiler lifecycle and
keep track of template changes so that it knows when to override them.
With this change, we no longer need the method `getModifiedResourceFile()`
on the adapter. Instead, we call `overrideComponentTemplate` on the
template type checker.

This commit also changes the incremental build strategy from
`PatchedIncrementalBuildStrategy` to `TrackedIncrementalBuildStrategy`.

PR Close #39231

packages/language-service/ivy/compiler_factory.ts

@@ -0,0 +1,75 @@
+/**
+ * @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 {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
+import {NgCompilerOptions} from '@angular/compiler-cli/src/ngtsc/core/api';
+import {TrackedIncrementalBuildStrategy} from '@angular/compiler-cli/src/ngtsc/incremental';
+import {TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
+import * as ts from 'typescript/lib/tsserverlibrary';
+
+import {isExternalTemplate, LanguageServiceAdapter} from './language_service_adapter';
+
+export class CompilerFactory {
+  private readonly incrementalStrategy = new TrackedIncrementalBuildStrategy();
+  private compiler: NgCompiler|null = null;
+  private lastKnownProgram: ts.Program|null = null;
+
+  constructor(
+      private readonly adapter: LanguageServiceAdapter,
+      private readonly programStrategy: TypeCheckingProgramStrategy,
+  ) {}
+
+  /**
+   * Create a new instance of the Ivy compiler if the program has changed since
+   * the last time the compiler was instantiated. If the program has not changed,
+   * return the existing instance.
+   * @param fileName override the template if this is an external template file
+   * @param options angular compiler options
+   */
+  getOrCreateWithChangedFile(fileName: string, options: NgCompilerOptions): NgCompiler {
+    const program = this.programStrategy.getProgram();
+    if (!this.compiler || program !== this.lastKnownProgram) {
+      this.compiler = new NgCompiler(
+          this.adapter,  // like compiler host
+          options,       // angular compiler options
+          program,
+          this.programStrategy,
+          this.incrementalStrategy,
+          true,  // enableTemplateTypeChecker
+          this.lastKnownProgram,
+          undefined,  // perfRecorder (use default)
+      );
+      this.lastKnownProgram = program;
+    }
+    if (isExternalTemplate(fileName)) {
+      this.overrideTemplate(fileName, this.compiler);
+    }
+    return this.compiler;
+  }
+
+  private overrideTemplate(fileName: string, compiler: NgCompiler) {
+    if (!this.adapter.isTemplateDirty(fileName)) {
+      return;
+    }
+    // 1. Get the latest snapshot
+    const latestTemplate = this.adapter.readResource(fileName);
+    // 2. Find all components that use the template
+    const ttc = compiler.getTemplateTypeChecker();
+    const components = compiler.getComponentsWithTemplateFile(fileName);
+    // 3. Update component template
+    for (const component of components) {
+      if (ts.isClassDeclaration(component)) {
+        ttc.overrideComponentTemplate(component, latestTemplate);
+      }
+    }
+  }
+
+  registerLastKnownProgram() {
+    this.lastKnownProgram = this.programStrategy.getProgram();
+  }
+}

packages/language-service/ivy/language_service.ts

@@ -9,18 +9,18 @@
 import {CompilerOptions, createNgCompilerOptions} from '@angular/compiler-cli';
 import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
 import {absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
-import {PatchedProgramIncrementalBuildStrategy} from '@angular/compiler-cli/src/ngtsc/incremental';
 import {TypeCheckShimGenerator} from '@angular/compiler-cli/src/ngtsc/typecheck';
 import {OptimizeFor, TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
 import * as ts from 'typescript/lib/tsserverlibrary';
 
+import {CompilerFactory} from './compiler_factory';
 import {DefinitionBuilder} from './definitions';
 import {isExternalTemplate, isTypeScriptFile, LanguageServiceAdapter} from './language_service_adapter';
 import {QuickInfoBuilder} from './quick_info';
 
 export class LanguageService {
   private options: CompilerOptions;
-  private lastKnownProgram: ts.Program|null = null;
+  private readonly compilerFactory: CompilerFactory;
   private readonly strategy: TypeCheckingProgramStrategy;
   private readonly adapter: LanguageServiceAdapter;
 
@@ -28,15 +28,16 @@ export class LanguageService {
     this.options = parseNgCompilerOptions(project);
     this.strategy = createTypeCheckingProgramStrategy(project);
     this.adapter = new LanguageServiceAdapter(project);
+    this.compilerFactory = new CompilerFactory(this.adapter, this.strategy);
     this.watchConfigFile(project);
   }
 
   getSemanticDiagnostics(fileName: string): ts.Diagnostic[] {
-    const program = this.strategy.getProgram();
-    const compiler = this.createCompiler(program, fileName);
+    const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName, this.options);
     const ttc = compiler.getTemplateTypeChecker();
     const diagnostics: ts.Diagnostic[] = [];
     if (isTypeScriptFile(fileName)) {
+      const program = compiler.getNextProgram();
       const sourceFile = program.getSourceFile(fileName);
       if (sourceFile) {
         diagnostics.push(...ttc.getDiagnosticsForFile(sourceFile, OptimizeFor.SingleFile));
@@ -49,51 +50,33 @@ export class LanguageService {
         }
       }
     }
-    this.lastKnownProgram = compiler.getNextProgram();
+    this.compilerFactory.registerLastKnownProgram();
     return diagnostics;
   }
 
   getDefinitionAndBoundSpan(fileName: string, position: number): ts.DefinitionInfoAndBoundSpan
       |undefined {
-    const program = this.strategy.getProgram();
-    const compiler = this.createCompiler(program, fileName);
-    return new DefinitionBuilder(this.tsLS, compiler).getDefinitionAndBoundSpan(fileName, position);
+    const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName, this.options);
+    const results =
+        new DefinitionBuilder(this.tsLS, compiler).getDefinitionAndBoundSpan(fileName, position);
+    this.compilerFactory.registerLastKnownProgram();
+    return results;
   }
 
   getTypeDefinitionAtPosition(fileName: string, position: number):
       readonly ts.DefinitionInfo[]|undefined {
-    const program = this.strategy.getProgram();
-    const compiler = this.createCompiler(program, fileName);
-    return new DefinitionBuilder(this.tsLS, compiler)
-        .getTypeDefinitionsAtPosition(fileName, position);
+    const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName, this.options);
+    const results =
+        new DefinitionBuilder(this.tsLS, compiler).getTypeDefinitionsAtPosition(fileName, position);
+    this.compilerFactory.registerLastKnownProgram();
+    return results;
   }
 
   getQuickInfoAtPosition(fileName: string, position: number): ts.QuickInfo|undefined {
-    const program = this.strategy.getProgram();
-    const compiler = this.createCompiler(program, fileName);
-    return new QuickInfoBuilder(this.tsLS, compiler).get(fileName, position);
-  }
-
-  /**
-   * Create a new instance of Ivy compiler.
-   * If the specified `fileName` refers to an external template, check if it has
-   * changed since the last time it was read. If it has changed, signal the
-   * compiler to reload the file via the adapter.
-   */
-  private createCompiler(program: ts.Program, fileName: string): NgCompiler {
-    if (isExternalTemplate(fileName)) {
-      this.adapter.registerTemplateUpdate(fileName);
-    }
-    return new NgCompiler(
-        this.adapter,
-        this.options,
-        program,
-        this.strategy,
-        new PatchedProgramIncrementalBuildStrategy(),
-        /** enableTemplateTypeChecker */ true,
-        this.lastKnownProgram,
-        /** perfRecorder (use default) */ undefined,
-    );
+    const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName, this.options);
+    const results = new QuickInfoBuilder(this.tsLS, compiler).get(fileName, position);
+    this.compilerFactory.registerLastKnownProgram();
+    return results;
   }
 
   private watchConfigFile(project: ts.server.Project) {

packages/language-service/ivy/language_service_adapter.ts

@@ -19,7 +19,6 @@ export class LanguageServiceAdapter implements NgCompilerAdapter {
   readonly unifiedModulesHost = null;  // only used in Bazel
   readonly rootDirs: AbsoluteFsPath[];
   private readonly templateVersion = new Map<string, string>();
-  private readonly modifiedTemplates = new Set<string>();
 
   constructor(private readonly project: ts.server.Project) {
     this.rootDirs = project.getCompilationSettings().rootDirs?.map(absoluteFrom) || [];
@@ -68,37 +67,13 @@ export class LanguageServiceAdapter implements NgCompilerAdapter {
     }
     const version = this.project.getScriptVersion(fileName);
     this.templateVersion.set(fileName, version);
-    this.modifiedTemplates.delete(fileName);
     return snapshot.getText(0, snapshot.getLength());
   }
 
-  /**
-   * getModifiedResourceFiles() is an Angular-specific method for notifying
-   * the Angular compiler templates that have changed since it last read them.
-   * It is a method on ExtendedTsCompilerHost, see
-   * packages/compiler-cli/src/ngtsc/core/api/src/interfaces.ts
-   */
-  getModifiedResourceFiles(): Set<string> {
-    return this.modifiedTemplates;
-  }
-
-  /**
-   * Check whether the specified `fileName` is newer than the last time it was
-   * read. If it is newer, register it and return true, otherwise do nothing and
-   * return false.
-   * @param fileName path to external template
-   */
-  registerTemplateUpdate(fileName: string): boolean {
-    if (!isExternalTemplate(fileName)) {
-      return false;
-    }
+  isTemplateDirty(fileName: string): boolean {
     const lastVersion = this.templateVersion.get(fileName);
     const latestVersion = this.project.getScriptVersion(fileName);
-    if (lastVersion !== latestVersion) {
-      this.modifiedTemplates.add(fileName);
-      return true;
-    }
-    return false;
+    return lastVersion !== latestVersion;
   }
 }
 

packages/language-service/ivy/test/compiler_factory_spec.ts

@@ -0,0 +1,78 @@
+/**
+ * @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 {APP_COMPONENT, setup, TEST_TEMPLATE} from './mock_host';
+
+const {project, service} = setup();
+
+/**
+ * The following specs do not directly test the CompilerFactory class, rather
+ * it asserts our understanding of the project/program/template interaction
+ * which forms the underlying assumption used in the CompilerFactory.
+ */
+
+describe('tsserver', () => {
+  beforeEach(() => {
+    service.reset();
+  });
+
+  describe('project version', () => {
+    it('is updated after TS changes', () => {
+      const v0 = project.getProjectVersion();
+      service.overwriteInlineTemplate(APP_COMPONENT, `{{ foo }}`);
+      const v1 = project.getProjectVersion();
+      expect(v1).not.toBe(v0);
+    });
+
+    it('is updated after template changes', () => {
+      const v0 = project.getProjectVersion();
+      service.overwrite(TEST_TEMPLATE, `{{ bar }}`);
+      const v1 = project.getProjectVersion();
+      expect(v1).not.toBe(v0);
+    });
+  });
+
+  describe('program', () => {
+    it('is updated after TS changes', () => {
+      const p0 = project.getLanguageService().getProgram();
+      expect(p0).toBeDefined();
+      service.overwriteInlineTemplate(APP_COMPONENT, `{{ foo }}`);
+      const p1 = project.getLanguageService().getProgram();
+      expect(p1).not.toBe(p0);
+    });
+
+    it('is not updated after template changes', () => {
+      const p0 = project.getLanguageService().getProgram();
+      expect(p0).toBeDefined();
+      service.overwrite(TEST_TEMPLATE, `{{ bar }}`);
+      const p1 = project.getLanguageService().getProgram();
+      expect(p1).toBe(p0);
+    });
+  });
+
+  describe('external template', () => {
+    it('should not be part of the project root', () => {
+      // Templates should _never_ be added to the project root, otherwise they
+      // will be parsed as TS files.
+      const scriptInfo = service.getScriptInfo(TEST_TEMPLATE);
+      expect(project.isRoot(scriptInfo)).toBeFalse();
+    });
+
+    it('should be attached to project', () => {
+      const scriptInfo = service.getScriptInfo(TEST_TEMPLATE);
+      // Script info for external template must be attached to a project because
+      // that's the primary mechanism used in the extension to locate the right
+      // language service instance. See
+      // https://github.com/angular/vscode-ng-language-service/blob/b048f5f6b9996c5cf113b764c7ffe13d9eeb4285/server/src/session.ts#L192
+      expect(scriptInfo.isAttached(project)).toBeTrue();
+      // Attaching a script info to a project does not mean that the project
+      // will contain the script info. Kinda like friendship on social media.
+      expect(project.containsScriptInfo(scriptInfo)).toBeFalse();
+    });
+  });
+});

packages/language-service/ivy/test/language_service_adapter_spec.ts

@@ -12,38 +12,20 @@ import {setup, TEST_TEMPLATE} from './mock_host';
 const {project, service} = setup();
 
 describe('Language service adapter', () => {
-  it('should register update if it has not seen the template before', () => {
+  it('should mark template dirty if it has not seen the template before', () => {
     const adapter = new LanguageServiceAdapter(project);
-    // Note that readResource() has never been called, so the adapter has no
-    // knowledge of the template at all.
-    const isRegistered = adapter.registerTemplateUpdate(TEST_TEMPLATE);
-    expect(isRegistered).toBeTrue();
-    expect(adapter.getModifiedResourceFiles().size).toBe(1);
+    expect(adapter.isTemplateDirty(TEST_TEMPLATE)).toBeTrue();
   });
 
-  it('should not register update if template has not changed', () => {
+  it('should not mark template dirty if template has not changed', () => {
     const adapter = new LanguageServiceAdapter(project);
     adapter.readResource(TEST_TEMPLATE);
-    const isRegistered = adapter.registerTemplateUpdate(TEST_TEMPLATE);
-    expect(isRegistered).toBeFalse();
-    expect(adapter.getModifiedResourceFiles().size).toBe(0);
+    expect(adapter.isTemplateDirty(TEST_TEMPLATE)).toBeFalse();
   });
 
-  it('should register update if template has changed', () => {
+  it('should mark template dirty if template has changed', () => {
     const adapter = new LanguageServiceAdapter(project);
-    adapter.readResource(TEST_TEMPLATE);
     service.overwrite(TEST_TEMPLATE, '<p>Hello World</p>');
-    const isRegistered = adapter.registerTemplateUpdate(TEST_TEMPLATE);
-    expect(isRegistered).toBe(true);
-    expect(adapter.getModifiedResourceFiles().size).toBe(1);
-  });
-
-  it('should clear template updates on read', () => {
-    const adapter = new LanguageServiceAdapter(project);
-    const isRegistered = adapter.registerTemplateUpdate(TEST_TEMPLATE);
-    expect(isRegistered).toBeTrue();
-    expect(adapter.getModifiedResourceFiles().size).toBe(1);
-    adapter.readResource(TEST_TEMPLATE);
-    expect(adapter.getModifiedResourceFiles().size).toBe(0);
+    expect(adapter.isTemplateDirty(TEST_TEMPLATE)).toBeTrue();
   });
 });

packages/language-service/ivy/test/language_service_spec.ts

@@ -6,11 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {parseNgCompilerOptions} from '../language_service';
+import {LanguageService, parseNgCompilerOptions} from '../language_service';
 
-import {setup} from './mock_host';
+import {setup, TEST_TEMPLATE} from './mock_host';
 
-const {project} = setup();
+const {project, tsLS, service} = setup();
 
 describe('parseNgCompilerOptions', () => {
   it('should read angularCompilerOptions in tsconfig.json', () => {
@@ -22,3 +22,91 @@ describe('parseNgCompilerOptions', () => {
     }));
   });
 });
+
+describe('last known program', () => {
+  const ngLS = new LanguageService(project, tsLS);
+
+  beforeEach(() => {
+    service.reset();
+  });
+
+  it('should be set after getSemanticDiagnostics()', () => {
+    const d0 = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
+    expect(d0.length).toBe(0);
+    const p0 = getLastKnownProgram(ngLS);
+
+    const d1 = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
+    expect(d1.length).toBe(0);
+    const p1 = getLastKnownProgram(ngLS);
+    expect(p1).toBe(p0);  // last known program should not have changed
+
+    service.overwrite(TEST_TEMPLATE, `<test-c¦omp></test-comp>`);
+    const d2 = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
+    expect(d2.length).toBe(0);
+    const p2 = getLastKnownProgram(ngLS);
+    expect(p2).not.toBe(p1);  // last known program should have changed
+  });
+
+  it('should be set after getDefinitionAndBoundSpan()', () => {
+    const {position: pos0} = service.overwrite(TEST_TEMPLATE, `<test-c¦omp></test-comp>`);
+
+    const d0 = ngLS.getDefinitionAndBoundSpan(TEST_TEMPLATE, pos0);
+    expect(d0).toBeDefined();
+    const p0 = getLastKnownProgram(ngLS);
+
+    const d1 = ngLS.getDefinitionAndBoundSpan(TEST_TEMPLATE, pos0);
+    expect(d1).toBeDefined();
+    const p1 = getLastKnownProgram(ngLS);
+    expect(p1).toBe(p0);  // last known program should not have changed
+
+    const {position: pos1} = service.overwrite(TEST_TEMPLATE, `{{ ti¦tle }}`);
+    const d2 = ngLS.getDefinitionAndBoundSpan(TEST_TEMPLATE, pos1);
+    expect(d2).toBeDefined();
+    const p2 = getLastKnownProgram(ngLS);
+    expect(p2).not.toBe(p1);  // last known program should have changed
+  });
+
+  it('should be set after getQuickInfoAtPosition()', () => {
+    const {position: pos0} = service.overwrite(TEST_TEMPLATE, `<test-c¦omp></test-comp>`);
+
+    const q0 = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, pos0);
+    expect(q0).toBeDefined();
+    const p0 = getLastKnownProgram(ngLS);
+
+    const q1 = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, pos0);
+    expect(q1).toBeDefined();
+    const p1 = getLastKnownProgram(ngLS);
+    expect(p1).toBe(p0);  // last known program should not have changed
+
+    const {position: pos1} = service.overwrite(TEST_TEMPLATE, `{{ ti¦tle }}`);
+    const q2 = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, pos1);
+    expect(q2).toBeDefined();
+    const p2 = getLastKnownProgram(ngLS);
+    expect(p2).not.toBe(p1);  // last known program should have changed
+  });
+
+  it('should be set after getTypeDefinitionAtPosition()', () => {
+    const {position: pos0} = service.overwrite(TEST_TEMPLATE, `<test-c¦omp></test-comp>`);
+
+    const q0 = ngLS.getTypeDefinitionAtPosition(TEST_TEMPLATE, pos0);
+    expect(q0).toBeDefined();
+    const p0 = getLastKnownProgram(ngLS);
+
+    const d1 = ngLS.getTypeDefinitionAtPosition(TEST_TEMPLATE, pos0);
+    expect(d1).toBeDefined();
+    const p1 = getLastKnownProgram(ngLS);
+    expect(p1).toBe(p0);  // last known program should not have changed
+
+    const {position: pos1} = service.overwrite(TEST_TEMPLATE, `{{ ti¦tle }}`);
+    const d2 = ngLS.getTypeDefinitionAtPosition(TEST_TEMPLATE, pos1);
+    expect(d2).toBeDefined();
+    const p2 = getLastKnownProgram(ngLS);
+    expect(p2).not.toBe(p1);  // last known program should have changed
+  });
+});
+
+function getLastKnownProgram(ngLS: LanguageService): ts.Program {
+  const program = ngLS['compilerFactory']['lastKnownProgram'];
+  expect(program).toBeDefined();
+  return program!;
+}

packages/language-service/ivy/test/mock_host.ts

@@ -158,6 +158,8 @@ export class MockService {
       }
     }
     this.overwritten.clear();
+    // updateGraph() will clear the internal dirty flag.
+    this.project.updateGraph();
   }
 
   getScriptInfo(fileName: string): ts.server.ScriptInfo {
@@ -179,6 +181,7 @@ export class MockService {
     if (!newScriptInfo) {
       throw new Error(`Failed to create new script info for ${fileName}`);
     }
+    newScriptInfo.attachToProject(this.project);
     return newScriptInfo;
   }