Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(language-service): Use tsLSHost.fileExists() to resolve modules #32642

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 40 additions & 16 deletions packages/language-service/src/reflector_host.ts
Expand Up @@ -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);
ayazhafiz marked this conversation as resolved.
Show resolved Hide resolved
}
}

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;
Expand Down
15 changes: 7 additions & 8 deletions packages/language-service/test/reflector_host_spec.ts
Expand Up @@ -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);

Expand All @@ -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 |
Expand Down