Skip to content
Permalink
Browse files

fix(language-service): Invalidate Reflector caches when program chang…

…es (#32357)

This commit fixes a bug introduced in the recent refactoring whereby
caches become stale when the program changes.
This is because StaticReflector keeps its own caches that are not
clearable. The previous refactoring tried to reuse the same instance,
leading to out-of-sync program state.

Clearing out the *entire* cache is very inefficient. Instead, we could
just invalidate the symbols in the files that have changed. This
requires changes to the API of StaticReflector, but put this on hold
until the refactoring of language service for Ivy commences.

PR closes #32301

PR Close #32357
  • Loading branch information...
kyliau authored and mhevery committed Aug 27, 2019
1 parent 4b12511 commit 97fc45f32a3e0a091636016bf863fd147105178c
@@ -57,8 +57,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
private readonly summaryResolver: AotSummaryResolver;
private readonly reflectorHost: ReflectorHost;
private readonly staticSymbolResolver: StaticSymbolResolver;
private readonly reflector: StaticReflector;
private readonly resolver: CompileMetadataResolver;
private resolver: CompileMetadataResolver;

private readonly staticSymbolCache = new StaticSymbolCache();
private readonly fileToComponent = new Map<string, StaticSymbol>();
@@ -82,28 +81,31 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
fromSummaryFileName(filePath: string): string{return filePath;},
},
this.staticSymbolCache);
this.reflectorHost = new ReflectorHost(() => tsLS.getProgram() !, host);
this.reflectorHost = new ReflectorHost(() => this.program, host);
this.staticSymbolResolver = new StaticSymbolResolver(
this.reflectorHost, this.staticSymbolCache, this.summaryResolver,
(e, filePath) => this.collectError(e, filePath));
this.reflector = new StaticReflector(
this.summaryResolver, this.staticSymbolResolver,
[], // knownMetadataClasses
[], // knownMetadataFunctions
(e, filePath) => this.collectError(e, filePath));
this.resolver = this.createMetadataResolver();
}

/**
* Creates a new metadata resolver. This should only be called once.
* Creates a new metadata resolver. This is needed whenever the program
* changes.
*/
private createMetadataResolver(): CompileMetadataResolver {
if (this.resolver) {
return this.resolver; // There should only be a single instance
}
const moduleResolver = new NgModuleResolver(this.reflector);
const directiveResolver = new DirectiveResolver(this.reflector);
const pipeResolver = new PipeResolver(this.reflector);
// 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.
const staticReflector = new StaticReflector(
this.summaryResolver, this.staticSymbolResolver,
[], // knownMetadataClasses
[], // knownMetadataFunctions
(e, filePath) => this.collectError(e, filePath));
// Because static reflector above is changed, we need to create a new
// resolver.
const moduleResolver = new NgModuleResolver(staticReflector);
const directiveResolver = new DirectiveResolver(staticReflector);
const pipeResolver = new PipeResolver(staticReflector);
const elementSchemaRegistry = new DomElementSchemaRegistry();
const resourceLoader = new DummyResourceLoader();
const urlResolver = createOfflineCompileUrlResolver();
@@ -119,7 +121,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
return new CompileMetadataResolver(
config, htmlParser, moduleResolver, directiveResolver, pipeResolver,
new JitSummaryResolver(), elementSchemaRegistry, directiveNormalizer, new Console(),
this.staticSymbolCache, this.reflector,
this.staticSymbolCache, staticReflector,
(error, type) => this.collectError(error, type && type.filePath));
}

@@ -141,6 +143,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
this.templateReferences = [];
this.fileToComponent.clear();
this.collectedErrors.clear();
this.resolver = this.createMetadataResolver();

const analyzeHost = {isSourceFile(filePath: string) { return true; }};
const programFiles = this.program.getSourceFiles().map(sf => sf.fileName);
@@ -247,7 +250,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
return this.program.getSourceFile(fileName);
}

get program() {
get program(): ts.Program {
const program = this.tsLS.getProgram();
if (!program) {
// Program is very very unlikely to be undefined.
@@ -256,6 +259,8 @@ 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.
@@ -95,4 +95,57 @@ describe('TypeScriptServiceHost', () => {
const template = templates[0];
expect(template.source).toContain('<h2>{{hero.name}} details!</h2>');
});

// https://github.com/angular/angular/issues/32301
it('should clear caches when program changes', () => {
const tsLSHost = new MockTypescriptHost(['/app/main.ts'], toh);
const tsLS = ts.createLanguageService(tsLSHost);
const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS);
const fileName = '/app/app.component.ts';

// Get initial state
const oldModules = ngLSHost.getAnalyzedModules();
// First, make sure there is no missing modules
expect(oldModules.symbolsMissingModule).toEqual([]);
// Expect to find AppComponent in the old modules
const oldFile = oldModules.files.find(f => f.fileName === fileName);
expect(oldFile !.directives.length).toBe(1);
const appComp = oldFile !.directives[0];
expect(appComp.name).toBe('AppComponent');
expect(oldModules.ngModuleByPipeOrDirective.has(appComp)).toBe(true);

// Now, override app.component.ts with a different component
tsLSHost.override(fileName, `
import {Component} from '@angular/core';
@Component({
template: '<div>Hello</div>
})
export class HelloComponent {}
`);
// And override the containing NgModule to import the new component
tsLSHost.override('/app/main.ts', `
import {NgModule} from '@angular/core';
import {HelloComponent} from './app.component';
@NgModule({
declarations: [
HelloComponent,
]
})
export class AppModule {}
`);
// Get the new state
const newModules = ngLSHost.getAnalyzedModules();
// Make sure there's no missing modules. If caches are not cleared properly,
// it will be a non-empty array
expect(newModules.symbolsMissingModule).toEqual([]);
// Expect to find HelloComponent in the new modules
const newFile = newModules.files.find(f => f.fileName === fileName);
expect(newFile !.directives.length).toBe(1);
const helloComp = newFile !.directives[0];
expect(helloComp.name).toBe('HelloComponent');
expect(newModules.ngModuleByPipeOrDirective.has(helloComp)).toBe(true);
expect(newModules.ngModuleByPipeOrDirective.has(appComp)).toBe(false);
});
});

0 comments on commit 97fc45f

Please sign in to comment.
You can’t perform that action at this time.