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): Lazily instantiate MetadataResolver #32631

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
35 changes: 26 additions & 9 deletions packages/language-service/src/typescript_host.ts
Expand Up @@ -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>();
Expand Down Expand Up @@ -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.
Expand All @@ -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]; }
Expand All @@ -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);
Expand Down Expand Up @@ -260,8 +279,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
return program;
}

get reflector(): StaticReflector { return this.resolver.getReflector() as StaticReflector; }
kyliau marked this conversation as resolved.
Show resolved Hide resolved

/**
* Checks whether the program has changed, and invalidate caches if it has.
* Returns true if modules are up-to-date, false otherwise.
Expand Down
8 changes: 3 additions & 5 deletions packages/language-service/test/reflector_host_spec.ts
Expand Up @@ -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.
Expand Down