From 1771d6ff25ff0d244044f317c05e341e9cccbdd1 Mon Sep 17 00:00:00 2001
From: Keen Yee Liau <kyliau@umich.edu>
Date: Wed, 11 Sep 2019 23:12:58 -0700
Subject: [PATCH] fix(language-service): Lazily instantiate MetadataResolver
 (#32631)

The instantiation of the resolver also requires instantiation of the
StaticReflector, and the latter requires resolution of core Angular symbols.
Module resolution should not be done during instantiation to avoid potential
cyclic dependency between the plugin and the containing Project, so the
Singleton pattern is used to create the resolver.

PR Close #32631
---
 .../language-service/src/typescript_host.ts   | 35 ++++++++++++++-----
 .../test/reflector_host_spec.ts               |  8 ++---
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts
index aa613432dc..9fc09b9e8b 100644
--- a/packages/language-service/src/typescript_host.ts
+++ b/packages/language-service/src/typescript_host.ts
@@ -57,7 +57,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
   private readonly summaryResolver: AotSummaryResolver;
   private readonly reflectorHost: ReflectorHost;
   private readonly staticSymbolResolver: StaticSymbolResolver;
-  private resolver: CompileMetadataResolver;
 
   private readonly staticSymbolCache = new StaticSymbolCache();
   private readonly fileToComponent = new Map<string, StaticSymbol>();
@@ -86,14 +85,23 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     this.staticSymbolResolver = new StaticSymbolResolver(
         this.reflectorHost, this.staticSymbolCache, this.summaryResolver,
         (e, filePath) => this.collectError(e, filePath));
-    this.resolver = this.createMetadataResolver();
   }
 
+  // The resolver is instantiated lazily and should not be accessed directly.
+  // Instead, call the resolver getter. The instantiation of the resolver also
+  // requires instantiation of the StaticReflector, and the latter requires
+  // resolution of core Angular symbols. Module resolution should not be done
+  // during instantiation to avoid cyclic dependency between the plugin and the
+  // containing Project, so the Singleton pattern is used here.
+  private _resolver: CompileMetadataResolver|undefined;
+
   /**
-   * Creates a new metadata resolver. This is needed whenever the program
-   * changes.
+   * Return the singleton instance of the MetadataResolver.
    */
-  private createMetadataResolver(): CompileMetadataResolver {
+  private get resolver(): CompileMetadataResolver {
+    if (this._resolver) {
+      return this._resolver;
+    }
     // StaticReflector keeps its own private caches that are not clearable.
     // We have no choice but to create a new instance to invalidate the caches.
     // TODO: Revisit this when language service gets rewritten for Ivy.
@@ -119,11 +127,20 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     });
     const directiveNormalizer =
         new DirectiveNormalizer(resourceLoader, urlResolver, htmlParser, config);
-    return new CompileMetadataResolver(
+    this._resolver = new CompileMetadataResolver(
         config, htmlParser, moduleResolver, directiveResolver, pipeResolver,
         new JitSummaryResolver(), elementSchemaRegistry, directiveNormalizer, new Console(),
         this.staticSymbolCache, staticReflector,
         (error, type) => this.collectError(error, type && type.filePath));
+    return this._resolver;
+  }
+
+  /**
+   * Return the singleton instance of the StaticReflector hosted in the
+   * MetadataResolver.
+   */
+  private get reflector(): StaticReflector {
+    return this.resolver.getReflector() as StaticReflector;
   }
 
   getTemplateReferences(): string[] { return [...this.templateReferences]; }
@@ -144,7 +161,9 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     this.templateReferences = [];
     this.fileToComponent.clear();
     this.collectedErrors.clear();
-    this.resolver = this.createMetadataResolver();
+    // TODO: This is only temporary. When https://github.com/angular/angular/pull/32543
+    // is merged this is no longer necessary.
+    this._resolver = undefined;  // Invalidate the resolver
 
     const analyzeHost = {isSourceFile(filePath: string) { return true; }};
     const programFiles = this.program.getSourceFiles().map(sf => sf.fileName);
@@ -260,8 +279,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     return program;
   }
 
-  get reflector(): StaticReflector { return this.resolver.getReflector() as StaticReflector; }
-
   /**
    * Checks whether the program has changed, and invalidate caches if it has.
    * Returns true if modules are up-to-date, false otherwise.
diff --git a/packages/language-service/test/reflector_host_spec.ts b/packages/language-service/test/reflector_host_spec.ts
index d370c8c51e..5215719d0a 100644
--- a/packages/language-service/test/reflector_host_spec.ts
+++ b/packages/language-service/test/reflector_host_spec.ts
@@ -51,13 +51,11 @@ describe('reflector_host_spec', () => {
 
     const tsLS = ts.createLanguageService(mockHost);
 
-    // First count is due to the instantiation of StaticReflector, which
-    // performs resolutions of core Angular symbols, like `NgModule`.
-    // TODO: Reduce this count to zero doing lazy instantiation.
+    // First count is zero due to lazy instantiation of the StaticReflector
+    // and MetadataResolver.
     const ngLSHost = new TypeScriptServiceHost(mockHost, tsLS);
     const firstCount = spy.calls.count();
-    expect(firstCount).toBeGreaterThan(20);
-    expect(firstCount).toBeLessThan(50);
+    expect(firstCount).toBe(0);
     spy.calls.reset();
 
     // Second count is due to resolution of the Tour of Heroes (toh) project.