From bbb2798d416edffa5435792ee98ce16029e22682 Mon Sep 17 00:00:00 2001
From: Keen Yee Liau <kyliau@google.com>
Date: Thu, 12 Sep 2019 09:29:41 -0700
Subject: [PATCH] fix(language-service): Use tsLSHost.fileExists() to resolve
 modules (#32642)

The ModuleResolutionHost implementation inside ReflectorHost currently
relies on reading the snapshot to determine if a file exists, and use
the snapshot to retrieve the file content.
It is more straightforward and efficient to use the already existing
method fileExists() instead.

At runtime, the TypeScript LanguageServiceHost is really a Project, so
both fileExists() and readFile() methods are defined.

As a micro-optimization, skip fs lookup for tsx files.

PR Close #32642
---
 .../language-service/src/reflector_host.ts    | 56 +++++++++++++------
 .../test/reflector_host_spec.ts               | 15 +++--
 2 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/packages/language-service/src/reflector_host.ts b/packages/language-service/src/reflector_host.ts
index 2fa04b97db..5e60375e6e 100644
--- a/packages/language-service/src/reflector_host.ts
+++ b/packages/language-service/src/reflector_host.ts
@@ -12,30 +12,54 @@ import * as path from 'path';
 import * as ts from 'typescript';
 
 class ReflectorModuleModuleResolutionHost implements ts.ModuleResolutionHost, MetadataReaderHost {
-  // Note: verboseInvalidExpressions is important so that
-  // the collector will collect errors instead of throwing
-  private metadataCollector = new MetadataCollector({verboseInvalidExpression: true});
+  private readonly metadataCollector = new MetadataCollector({
+    // Note: verboseInvalidExpressions is important so that
+    // the collector will collect errors instead of throwing
+    verboseInvalidExpression: true,
+  });
 
-  constructor(private host: ts.LanguageServiceHost, private getProgram: () => ts.Program) {
-    if (host.directoryExists)
-      this.directoryExists = directoryName => this.host.directoryExists !(directoryName);
+  readonly directoryExists?: (directoryName: string) => boolean;
+
+  constructor(
+      private readonly tsLSHost: ts.LanguageServiceHost,
+      private readonly getProgram: () => ts.Program) {
+    if (tsLSHost.directoryExists) {
+      this.directoryExists = directoryName => tsLSHost.directoryExists !(directoryName);
+    }
   }
 
-  fileExists(fileName: string): boolean { return !!this.host.getScriptSnapshot(fileName); }
+  fileExists(fileName: string): boolean {
+    // TypeScript resolution logic walks through the following sequence in order:
+    // package.json (read "types" field) -> .ts -> .tsx -> .d.ts
+    // For more info, see
+    // https://www.typescriptlang.org/docs/handbook/module-resolution.html
+    // For Angular specifically, we can skip .tsx lookup
+    if (fileName.endsWith('.tsx')) {
+      return false;
+    }
+    if (this.tsLSHost.fileExists) {
+      return this.tsLSHost.fileExists(fileName);
+    }
+    return !!this.tsLSHost.getScriptSnapshot(fileName);
+  }
 
   readFile(fileName: string): string {
-    let snapshot = this.host.getScriptSnapshot(fileName);
-    if (snapshot) {
-      return snapshot.getText(0, snapshot.getLength());
+    // readFile() is used by TypeScript to read package.json during module
+    // resolution, and it's used by Angular to read metadata.json during
+    // metadata resolution.
+    if (this.tsLSHost.readFile) {
+      return this.tsLSHost.readFile(fileName) !;
     }
-
-    // Typescript readFile() declaration should be `readFile(fileName: string): string | undefined
-    return undefined !;
+    // As a fallback, read the JSON files from the editor snapshot.
+    const snapshot = this.tsLSHost.getScriptSnapshot(fileName);
+    if (!snapshot) {
+      // MetadataReaderHost readFile() declaration should be
+      // `readFile(fileName: string): string | undefined`
+      return undefined !;
+    }
+    return snapshot.getText(0, snapshot.getLength());
   }
 
-  // TODO(issue/24571): remove '!'.
-  directoryExists !: (directoryName: string) => boolean;
-
   getSourceFileMetadata(fileName: string) {
     const sf = this.getProgram().getSourceFile(fileName);
     return sf ? this.metadataCollector.getMetadata(sf) : undefined;
diff --git a/packages/language-service/test/reflector_host_spec.ts b/packages/language-service/test/reflector_host_spec.ts
index 5215719d0a..e395a010f2 100644
--- a/packages/language-service/test/reflector_host_spec.ts
+++ b/packages/language-service/test/reflector_host_spec.ts
@@ -44,10 +44,9 @@ describe('reflector_host_spec', () => {
   it('should use module resolution cache', () => {
     const mockHost = new MockTypescriptHost(['/app/main.ts'], toh);
     // TypeScript relies on `ModuleResolutionHost.fileExists()` to perform
-    // module resolution, and ReflectorHost uses
-    // `LanguageServiceHost.getScriptSnapshot()` to implement `fileExists()`,
-    // so spy on this method to determine how many times it's called.
-    const spy = spyOn(mockHost, 'getScriptSnapshot').and.callThrough();
+    // module resolution, so spy on this method to determine how many times
+    // it's called.
+    const spy = spyOn(mockHost, 'fileExists').and.callThrough();
 
     const tsLS = ts.createLanguageService(mockHost);
 
@@ -62,16 +61,16 @@ describe('reflector_host_spec', () => {
     // This resolves all Angular directives in the project.
     ngLSHost.getAnalyzedModules();
     const secondCount = spy.calls.count();
-    expect(secondCount).toBeGreaterThan(500);
-    expect(secondCount).toBeLessThan(600);
+    expect(secondCount).toBeGreaterThan(700);
+    expect(secondCount).toBeLessThan(800);
     spy.calls.reset();
 
     // Third count is due to recompution after the program changes.
     mockHost.addCode('');  // this will mark project as dirty
     ngLSHost.getAnalyzedModules();
     const thirdCount = spy.calls.count();
-    expect(thirdCount).toBeGreaterThan(50);
-    expect(thirdCount).toBeLessThan(100);
+    expect(thirdCount).toBeGreaterThan(0);
+    expect(thirdCount).toBeLessThan(10);
 
     // Summary
     // |               | First Count | Second Count | Third Count |