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
This commit is contained in:
Keen Yee Liau 2019-09-12 09:29:41 -07:00 committed by Kara Erickson
parent 2bf5606bbe
commit bbb2798d41
2 changed files with 47 additions and 24 deletions

View File

@ -12,30 +12,54 @@ import * as path from 'path';
import * as ts from 'typescript'; import * as ts from 'typescript';
class ReflectorModuleModuleResolutionHost implements ts.ModuleResolutionHost, MetadataReaderHost { class ReflectorModuleModuleResolutionHost implements ts.ModuleResolutionHost, MetadataReaderHost {
// Note: verboseInvalidExpressions is important so that private readonly metadataCollector = new MetadataCollector({
// the collector will collect errors instead of throwing // Note: verboseInvalidExpressions is important so that
private metadataCollector = new MetadataCollector({verboseInvalidExpression: true}); // the collector will collect errors instead of throwing
verboseInvalidExpression: true,
});
constructor(private host: ts.LanguageServiceHost, private getProgram: () => ts.Program) { readonly directoryExists?: (directoryName: string) => boolean;
if (host.directoryExists)
this.directoryExists = directoryName => this.host.directoryExists !(directoryName); 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 { readFile(fileName: string): string {
let snapshot = this.host.getScriptSnapshot(fileName); // readFile() is used by TypeScript to read package.json during module
if (snapshot) { // resolution, and it's used by Angular to read metadata.json during
return snapshot.getText(0, snapshot.getLength()); // metadata resolution.
if (this.tsLSHost.readFile) {
return this.tsLSHost.readFile(fileName) !;
} }
// As a fallback, read the JSON files from the editor snapshot.
// Typescript readFile() declaration should be `readFile(fileName: string): string | undefined const snapshot = this.tsLSHost.getScriptSnapshot(fileName);
return undefined !; 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) { getSourceFileMetadata(fileName: string) {
const sf = this.getProgram().getSourceFile(fileName); const sf = this.getProgram().getSourceFile(fileName);
return sf ? this.metadataCollector.getMetadata(sf) : undefined; return sf ? this.metadataCollector.getMetadata(sf) : undefined;

View File

@ -44,10 +44,9 @@ describe('reflector_host_spec', () => {
it('should use module resolution cache', () => { it('should use module resolution cache', () => {
const mockHost = new MockTypescriptHost(['/app/main.ts'], toh); const mockHost = new MockTypescriptHost(['/app/main.ts'], toh);
// TypeScript relies on `ModuleResolutionHost.fileExists()` to perform // TypeScript relies on `ModuleResolutionHost.fileExists()` to perform
// module resolution, and ReflectorHost uses // module resolution, so spy on this method to determine how many times
// `LanguageServiceHost.getScriptSnapshot()` to implement `fileExists()`, // it's called.
// so spy on this method to determine how many times it's called. const spy = spyOn(mockHost, 'fileExists').and.callThrough();
const spy = spyOn(mockHost, 'getScriptSnapshot').and.callThrough();
const tsLS = ts.createLanguageService(mockHost); const tsLS = ts.createLanguageService(mockHost);
@ -62,16 +61,16 @@ describe('reflector_host_spec', () => {
// This resolves all Angular directives in the project. // This resolves all Angular directives in the project.
ngLSHost.getAnalyzedModules(); ngLSHost.getAnalyzedModules();
const secondCount = spy.calls.count(); const secondCount = spy.calls.count();
expect(secondCount).toBeGreaterThan(500); expect(secondCount).toBeGreaterThan(700);
expect(secondCount).toBeLessThan(600); expect(secondCount).toBeLessThan(800);
spy.calls.reset(); spy.calls.reset();
// Third count is due to recompution after the program changes. // Third count is due to recompution after the program changes.
mockHost.addCode(''); // this will mark project as dirty mockHost.addCode(''); // this will mark project as dirty
ngLSHost.getAnalyzedModules(); ngLSHost.getAnalyzedModules();
const thirdCount = spy.calls.count(); const thirdCount = spy.calls.count();
expect(thirdCount).toBeGreaterThan(50); expect(thirdCount).toBeGreaterThan(0);
expect(thirdCount).toBeLessThan(100); expect(thirdCount).toBeLessThan(10);
// Summary // Summary
// | | First Count | Second Count | Third Count | // | | First Count | Second Count | Third Count |