feat(core): support `ngOnDestroy` on providers of a directive.

Such providers also become eager as they will be instantiated
anyways on destruction.
This commit is contained in:
Tobias Bosch 2016-08-02 01:37:42 -07:00
parent ff3b71f7b3
commit c161ed415d
10 changed files with 87 additions and 42 deletions

View File

@ -14,11 +14,11 @@ import {StringMapWrapper} from '../src/facade/collection';
import {assertArrayOfStrings, assertInterpolationSymbols} from './assertions'; import {assertArrayOfStrings, assertInterpolationSymbols} from './assertions';
import * as cpl from './compile_metadata'; import * as cpl from './compile_metadata';
import {CompilerConfig} from './config'; import {CompilerConfig} from './config';
import {hasLifecycleHook} from './directive_lifecycle_reflector';
import {DirectiveResolver} from './directive_resolver'; import {DirectiveResolver} from './directive_resolver';
import {BaseException} from './facade/exceptions'; import {BaseException} from './facade/exceptions';
import {Type, isArray, isBlank, isPresent, isString, stringify} from './facade/lang'; import {Type, isArray, isBlank, isPresent, isString, stringify} from './facade/lang';
import {Identifiers, identifierToken} from './identifiers'; import {Identifiers, identifierToken} from './identifiers';
import {hasLifecycleHook} from './lifecycle_reflector';
import {NgModuleResolver} from './ng_module_resolver'; import {NgModuleResolver} from './ng_module_resolver';
import {PipeResolver} from './pipe_resolver'; import {PipeResolver} from './pipe_resolver';
import {ElementSchemaRegistry} from './schema/element_schema_registry'; import {ElementSchemaRegistry} from './schema/element_schema_registry';

View File

@ -478,10 +478,13 @@ function _resolveProviders(
sourceSpan)); sourceSpan));
} }
if (isBlank(resolvedProvider)) { if (isBlank(resolvedProvider)) {
const lifecycleHooks = provider.useClass ? provider.useClass.lifecycleHooks : []; const lifecycleHooks =
provider.token.identifier && provider.token.identifier instanceof CompileTypeMetadata ?
provider.token.identifier.lifecycleHooks :
[];
resolvedProvider = new ProviderAst( resolvedProvider = new ProviderAst(
provider.token, provider.multi, eager, [provider], providerType, lifecycleHooks, provider.token, provider.multi, eager || lifecycleHooks.length > 0, [provider],
sourceSpan); providerType, lifecycleHooks, sourceSpan);
targetProvidersByToken.add(provider.token, resolvedProvider); targetProvidersByToken.add(provider.token, resolvedProvider);
} else { } else {
if (!provider.multi) { if (!provider.multi) {

View File

@ -43,7 +43,7 @@ export class CompileElement extends CompileNode {
public appElement: o.ReadPropExpr; public appElement: o.ReadPropExpr;
public elementRef: o.Expression; public elementRef: o.Expression;
public injector: o.Expression; public injector: o.Expression;
private _instances = new CompileIdentifierMap<CompileTokenMetadata, o.Expression>(); public instances = new CompileIdentifierMap<CompileTokenMetadata, o.Expression>();
private _resolvedProviders: CompileIdentifierMap<CompileTokenMetadata, ProviderAst>; private _resolvedProviders: CompileIdentifierMap<CompileTokenMetadata, ProviderAst>;
private _queryCount = 0; private _queryCount = 0;
@ -52,7 +52,6 @@ export class CompileElement extends CompileNode {
public contentNodesByNgContentIndex: Array<o.Expression>[] = null; public contentNodesByNgContentIndex: Array<o.Expression>[] = null;
public embeddedView: CompileView; public embeddedView: CompileView;
public directiveInstances: o.Expression[];
public referenceTokens: {[key: string]: CompileTokenMetadata}; public referenceTokens: {[key: string]: CompileTokenMetadata};
constructor( constructor(
@ -66,10 +65,10 @@ export class CompileElement extends CompileNode {
references.forEach(ref => this.referenceTokens[ref.name] = ref.value); references.forEach(ref => this.referenceTokens[ref.name] = ref.value);
this.elementRef = o.importExpr(Identifiers.ElementRef).instantiate([this.renderNode]); this.elementRef = o.importExpr(Identifiers.ElementRef).instantiate([this.renderNode]);
this._instances.add(identifierToken(Identifiers.ElementRef), this.elementRef); this.instances.add(identifierToken(Identifiers.ElementRef), this.elementRef);
this.injector = o.THIS_EXPR.callMethod('injector', [o.literal(this.nodeIndex)]); this.injector = o.THIS_EXPR.callMethod('injector', [o.literal(this.nodeIndex)]);
this._instances.add(identifierToken(Identifiers.Injector), this.injector); this.instances.add(identifierToken(Identifiers.Injector), this.injector);
this._instances.add(identifierToken(Identifiers.Renderer), o.THIS_EXPR.prop('renderer')); this.instances.add(identifierToken(Identifiers.Renderer), o.THIS_EXPR.prop('renderer'));
if (this.hasViewContainer || this.hasEmbeddedView || isPresent(this.component)) { if (this.hasViewContainer || this.hasEmbeddedView || isPresent(this.component)) {
this._createAppElement(); this._createAppElement();
} }
@ -89,7 +88,7 @@ export class CompileElement extends CompileNode {
.toStmt(); .toStmt();
this.view.createMethod.addStmt(statement); this.view.createMethod.addStmt(statement);
this.appElement = o.THIS_EXPR.prop(fieldName); this.appElement = o.THIS_EXPR.prop(fieldName);
this._instances.add(identifierToken(Identifiers.AppElement), this.appElement); this.instances.add(identifierToken(Identifiers.AppElement), this.appElement);
} }
public createComponentFactoryResolver(entryComponents: CompileIdentifierMetadata[]) { public createComponentFactoryResolver(entryComponents: CompileIdentifierMetadata[]) {
@ -139,7 +138,7 @@ export class CompileElement extends CompileNode {
beforeChildren(): void { beforeChildren(): void {
if (this.hasViewContainer) { if (this.hasViewContainer) {
this._instances.add( this.instances.add(
identifierToken(Identifiers.ViewContainerRef), this.appElement.prop('vcRef')); identifierToken(Identifiers.ViewContainerRef), this.appElement.prop('vcRef'));
} }
@ -168,18 +167,16 @@ export class CompileElement extends CompileNode {
return convertValueToOutputAst(provider.useValue); return convertValueToOutputAst(provider.useValue);
} }
}); });
var propName = `_${resolvedProvider.token.name}_${this.nodeIndex}_${this._instances.size}`; var propName = `_${resolvedProvider.token.name}_${this.nodeIndex}_${this.instances.size}`;
var instance = createProviderProperty( var instance = createProviderProperty(
propName, resolvedProvider, providerValueExpressions, resolvedProvider.multiProvider, propName, resolvedProvider, providerValueExpressions, resolvedProvider.multiProvider,
resolvedProvider.eager, this); resolvedProvider.eager, this);
this._instances.add(resolvedProvider.token, instance); this.instances.add(resolvedProvider.token, instance);
}); });
this.directiveInstances = for (var i = 0; i < this._directives.length; i++) {
this._directives.map((directive) => this._instances.get(identifierToken(directive.type)));
for (var i = 0; i < this.directiveInstances.length; i++) {
var directiveInstance = this.directiveInstances[i];
var directive = this._directives[i]; var directive = this._directives[i];
var directiveInstance = this.instances.get(identifierToken(directive.type));
directive.queries.forEach((queryMeta) => { this._addQuery(queryMeta, directiveInstance); }); directive.queries.forEach((queryMeta) => { this._addQuery(queryMeta, directiveInstance); });
} }
var queriesWithReads: _QueryWithRead[] = []; var queriesWithReads: _QueryWithRead[] = [];
@ -193,7 +190,7 @@ export class CompileElement extends CompileNode {
var token = this.referenceTokens[varName]; var token = this.referenceTokens[varName];
var varValue: o.Expression; var varValue: o.Expression;
if (isPresent(token)) { if (isPresent(token)) {
varValue = this._instances.get(token); varValue = this.instances.get(token);
} else { } else {
varValue = this.renderNode; varValue = this.renderNode;
} }
@ -207,12 +204,12 @@ export class CompileElement extends CompileNode {
var value: o.Expression; var value: o.Expression;
if (isPresent(queryWithRead.read.identifier)) { if (isPresent(queryWithRead.read.identifier)) {
// query for an identifier // query for an identifier
value = this._instances.get(queryWithRead.read); value = this.instances.get(queryWithRead.read);
} else { } else {
// query for a reference // query for a reference
var token = this.referenceTokens[queryWithRead.read.value]; var token = this.referenceTokens[queryWithRead.read.value];
if (isPresent(token)) { if (isPresent(token)) {
value = this._instances.get(token); value = this.instances.get(token);
} else { } else {
value = this.elementRef; value = this.elementRef;
} }
@ -241,7 +238,7 @@ export class CompileElement extends CompileNode {
// Note: afterChildren is called after recursing into children. // Note: afterChildren is called after recursing into children.
// This is good so that an injector match in an element that is closer to a requesting element // This is good so that an injector match in an element that is closer to a requesting element
// matches first. // matches first.
var providerExpr = this._instances.get(resolvedProvider.token); var providerExpr = this.instances.get(resolvedProvider.token);
// Note: view providers are only visible on the injector of that element. // Note: view providers are only visible on the injector of that element.
// This is not fully correct as the rules during codegen don't allow a directive // This is not fully correct as the rules during codegen don't allow a directive
// to get hold of a view provdier on the same element. We still do this semantic // to get hold of a view provdier on the same element. We still do this semantic
@ -263,7 +260,7 @@ export class CompileElement extends CompileNode {
} }
getComponent(): o.Expression { getComponent(): o.Expression {
return isPresent(this.component) ? this._instances.get(identifierToken(this.component.type)) : return isPresent(this.component) ? this.instances.get(identifierToken(this.component.type)) :
null; null;
} }
@ -342,7 +339,7 @@ export class CompileElement extends CompileNode {
resolvedProvider.providerType === ProviderAstType.PrivateService) { resolvedProvider.providerType === ProviderAstType.PrivateService) {
return null; return null;
} }
result = this._instances.get(dep.token); result = this.instances.get(dep.token);
} }
} }
return result; return result;

View File

@ -9,6 +9,7 @@
import {CompileDirectiveMetadata} from '../compile_metadata'; import {CompileDirectiveMetadata} from '../compile_metadata';
import {ListWrapper, StringMapWrapper} from '../facade/collection'; import {ListWrapper, StringMapWrapper} from '../facade/collection';
import {StringWrapper, isBlank, isPresent} from '../facade/lang'; import {StringWrapper, isBlank, isPresent} from '../facade/lang';
import {identifierToken} from '../identifiers';
import * as o from '../output/output_ast'; import * as o from '../output/output_ast';
import {BoundEventAst, DirectiveAst} from '../template_parser/template_ast'; import {BoundEventAst, DirectiveAst} from '../template_parser/template_ast';
@ -134,8 +135,9 @@ export function collectEventListeners(
compileElement, hostEvent.target, hostEvent.name, eventListeners); compileElement, hostEvent.target, hostEvent.name, eventListeners);
listener.addAction(hostEvent, null, null); listener.addAction(hostEvent, null, null);
}); });
ListWrapper.forEachWithIndex(dirs, (directiveAst, i) => { dirs.forEach((directiveAst) => {
var directiveInstance = compileElement.directiveInstances[i]; var directiveInstance =
compileElement.instances.get(identifierToken(directiveAst.directive.type));
directiveAst.hostEvents.forEach((hostEvent) => { directiveAst.hostEvents.forEach((hostEvent) => {
compileElement.view.bindings.push(new CompileBinding(compileElement, hostEvent)); compileElement.view.bindings.push(new CompileBinding(compileElement, hostEvent));
var listener = CompileEventListener.getOrCreate( var listener = CompileEventListener.getOrCreate(

View File

@ -9,7 +9,7 @@
import {LifecycleHooks} from '../../core_private'; import {LifecycleHooks} from '../../core_private';
import {CompileDirectiveMetadata, CompilePipeMetadata} from '../compile_metadata'; import {CompileDirectiveMetadata, CompilePipeMetadata} from '../compile_metadata';
import * as o from '../output/output_ast'; import * as o from '../output/output_ast';
import {DirectiveAst} from '../template_parser/template_ast'; import {DirectiveAst, ProviderAst} from '../template_parser/template_ast';
import {CompileElement} from './compile_element'; import {CompileElement} from './compile_element';
import {CompileView} from './compile_view'; import {CompileView} from './compile_view';
@ -77,12 +77,11 @@ export function bindDirectiveAfterViewLifecycleCallbacks(
} }
} }
export function bindDirectiveDestroyLifecycleCallbacks( export function bindInjectableDestroyLifecycleCallbacks(
directiveMeta: CompileDirectiveMetadata, directiveInstance: o.Expression, provider: ProviderAst, directiveInstance: o.Expression, compileElement: CompileElement) {
compileElement: CompileElement) {
var onDestroyMethod = compileElement.view.destroyMethod; var onDestroyMethod = compileElement.view.destroyMethod;
onDestroyMethod.resetDebugInfo(compileElement.nodeIndex, compileElement.sourceAst); onDestroyMethod.resetDebugInfo(compileElement.nodeIndex, compileElement.sourceAst);
if (directiveMeta.type.lifecycleHooks.indexOf(LifecycleHooks.OnDestroy) !== -1) { if (provider.lifecycleHooks.indexOf(LifecycleHooks.OnDestroy) !== -1) {
onDestroyMethod.addStmt(directiveInstance.callMethod('ngOnDestroy', []).toStmt()); onDestroyMethod.addStmt(directiveInstance.callMethod('ngOnDestroy', []).toStmt());
} }
} }

View File

@ -10,9 +10,10 @@ import {ListWrapper,} from '../facade/collection';
import {TemplateAst, TemplateAstVisitor, NgContentAst, EmbeddedTemplateAst, ElementAst, ReferenceAst, VariableAst, BoundEventAst, BoundElementPropertyAst, AttrAst, BoundTextAst, TextAst, DirectiveAst, BoundDirectivePropertyAst, templateVisitAll,} from '../template_parser/template_ast'; import {TemplateAst, TemplateAstVisitor, NgContentAst, EmbeddedTemplateAst, ElementAst, ReferenceAst, VariableAst, BoundEventAst, BoundElementPropertyAst, AttrAst, BoundTextAst, TextAst, DirectiveAst, BoundDirectivePropertyAst, templateVisitAll,} from '../template_parser/template_ast';
import {bindRenderText, bindRenderInputs, bindDirectiveInputs, bindDirectiveHostProps} from './property_binder'; import {bindRenderText, bindRenderInputs, bindDirectiveInputs, bindDirectiveHostProps} from './property_binder';
import {bindRenderOutputs, collectEventListeners, bindDirectiveOutputs} from './event_binder'; import {bindRenderOutputs, collectEventListeners, bindDirectiveOutputs} from './event_binder';
import {bindDirectiveAfterContentLifecycleCallbacks, bindDirectiveAfterViewLifecycleCallbacks, bindDirectiveDestroyLifecycleCallbacks, bindPipeDestroyLifecycleCallbacks, bindDirectiveDetectChangesLifecycleCallbacks} from './lifecycle_binder'; import {bindDirectiveAfterContentLifecycleCallbacks, bindDirectiveAfterViewLifecycleCallbacks, bindInjectableDestroyLifecycleCallbacks, bindPipeDestroyLifecycleCallbacks, bindDirectiveDetectChangesLifecycleCallbacks} from './lifecycle_binder';
import {CompileView} from './compile_view'; import {CompileView} from './compile_view';
import {CompileElement, CompileNode} from './compile_element'; import {CompileElement, CompileNode} from './compile_element';
import {identifierToken} from '../identifiers';
export function bindView(view: CompileView, parsedTemplate: TemplateAst[]): void { export function bindView(view: CompileView, parsedTemplate: TemplateAst[]): void {
var visitor = new ViewBinderVisitor(view); var visitor = new ViewBinderVisitor(view);
@ -43,8 +44,9 @@ class ViewBinderVisitor implements TemplateAstVisitor {
var eventListeners = collectEventListeners(ast.outputs, ast.directives, compileElement); var eventListeners = collectEventListeners(ast.outputs, ast.directives, compileElement);
bindRenderInputs(ast.inputs, compileElement); bindRenderInputs(ast.inputs, compileElement);
bindRenderOutputs(eventListeners); bindRenderOutputs(eventListeners);
ListWrapper.forEachWithIndex(ast.directives, (directiveAst, index) => { ast.directives.forEach((directiveAst) => {
var directiveInstance = compileElement.directiveInstances[index]; var directiveInstance =
compileElement.instances.get(identifierToken(directiveAst.directive.type));
bindDirectiveInputs(directiveAst, directiveInstance, compileElement); bindDirectiveInputs(directiveAst, directiveInstance, compileElement);
bindDirectiveDetectChangesLifecycleCallbacks(directiveAst, directiveInstance, compileElement); bindDirectiveDetectChangesLifecycleCallbacks(directiveAst, directiveInstance, compileElement);
@ -54,14 +56,17 @@ class ViewBinderVisitor implements TemplateAstVisitor {
templateVisitAll(this, ast.children, compileElement); templateVisitAll(this, ast.children, compileElement);
// afterContent and afterView lifecycles need to be called bottom up // afterContent and afterView lifecycles need to be called bottom up
// so that children are notified before parents // so that children are notified before parents
ListWrapper.forEachWithIndex(ast.directives, (directiveAst, index) => { ast.directives.forEach((directiveAst) => {
var directiveInstance = compileElement.directiveInstances[index]; var directiveInstance =
compileElement.instances.get(identifierToken(directiveAst.directive.type));
bindDirectiveAfterContentLifecycleCallbacks( bindDirectiveAfterContentLifecycleCallbacks(
directiveAst.directive, directiveInstance, compileElement); directiveAst.directive, directiveInstance, compileElement);
bindDirectiveAfterViewLifecycleCallbacks( bindDirectiveAfterViewLifecycleCallbacks(
directiveAst.directive, directiveInstance, compileElement); directiveAst.directive, directiveInstance, compileElement);
bindDirectiveDestroyLifecycleCallbacks( });
directiveAst.directive, directiveInstance, compileElement); ast.providers.forEach((providerAst) => {
var providerInstance = compileElement.instances.get(providerAst.token);
bindInjectableDestroyLifecycleCallbacks(providerAst, providerInstance, compileElement);
}); });
return null; return null;
} }
@ -69,8 +74,9 @@ class ViewBinderVisitor implements TemplateAstVisitor {
visitEmbeddedTemplate(ast: EmbeddedTemplateAst, parent: CompileElement): any { visitEmbeddedTemplate(ast: EmbeddedTemplateAst, parent: CompileElement): any {
var compileElement = <CompileElement>this.view.nodes[this._nodeIndex++]; var compileElement = <CompileElement>this.view.nodes[this._nodeIndex++];
var eventListeners = collectEventListeners(ast.outputs, ast.directives, compileElement); var eventListeners = collectEventListeners(ast.outputs, ast.directives, compileElement);
ListWrapper.forEachWithIndex(ast.directives, (directiveAst, index) => { ast.directives.forEach((directiveAst) => {
var directiveInstance = compileElement.directiveInstances[index]; var directiveInstance =
compileElement.instances.get(identifierToken(directiveAst.directive.type));
bindDirectiveInputs(directiveAst, directiveInstance, compileElement); bindDirectiveInputs(directiveAst, directiveInstance, compileElement);
bindDirectiveDetectChangesLifecycleCallbacks(directiveAst, directiveInstance, compileElement); bindDirectiveDetectChangesLifecycleCallbacks(directiveAst, directiveInstance, compileElement);
bindDirectiveOutputs(directiveAst, directiveInstance, eventListeners); bindDirectiveOutputs(directiveAst, directiveInstance, eventListeners);
@ -78,8 +84,10 @@ class ViewBinderVisitor implements TemplateAstVisitor {
directiveAst.directive, directiveInstance, compileElement); directiveAst.directive, directiveInstance, compileElement);
bindDirectiveAfterViewLifecycleCallbacks( bindDirectiveAfterViewLifecycleCallbacks(
directiveAst.directive, directiveInstance, compileElement); directiveAst.directive, directiveInstance, compileElement);
bindDirectiveDestroyLifecycleCallbacks( });
directiveAst.directive, directiveInstance, compileElement); ast.providers.forEach((providerAst) => {
var providerInstance = compileElement.instances.get(providerAst.token);
bindInjectableDestroyLifecycleCallbacks(providerAst, providerInstance, compileElement);
}); });
bindView(compileElement.embeddedView, ast.children); bindView(compileElement.embeddedView, ast.children);
return null; return null;

View File

@ -8,7 +8,7 @@
import {beforeEach, xdescribe, ddescribe, describe, expect, iit, inject, it,} from '@angular/core/testing/testing_internal'; import {beforeEach, xdescribe, ddescribe, describe, expect, iit, inject, it,} from '@angular/core/testing/testing_internal';
import {hasLifecycleHook} from '@angular/compiler/src/directive_lifecycle_reflector'; import {hasLifecycleHook} from '@angular/compiler/src/lifecycle_reflector';
import {LifecycleHooks} from '@angular/core/src/metadata/lifecycle_hooks'; import {LifecycleHooks} from '@angular/core/src/metadata/lifecycle_hooks';
export function main() { export function main() {

View File

@ -1026,6 +1026,20 @@ export function main() {
'pipeWithOnDestroy.ngOnDestroy' 'pipeWithOnDestroy.ngOnDestroy'
]); ]);
})); }));
it('should call ngOnDestroy on an injectable class', fakeAsync(() => {
var ctx = createCompFixture(
'<div testDirective="dir"></div>', TestComponent,
tcb.overrideProviders(TestDirective, [InjectableWithLifecycle]));
ctx.debugElement.children[0].injector.get(InjectableWithLifecycle);
ctx.detectChanges(false);
ctx.destroy();
expect(directiveLog.filter(['ngOnDestroy'])).toEqual([
'dir.ngOnDestroy', 'injectable.ngOnDestroy'
]);
}));
}); });
}); });
@ -1386,6 +1400,14 @@ class TestDirective implements OnInit, DoCheck, OnChanges, AfterContentInit, Aft
} }
} }
@Injectable()
class InjectableWithLifecycle {
name = 'injectable';
constructor(public log: DirectiveLog) {}
ngOnDestroy() { this.log.add(this.name, 'ngOnDestroy'); }
}
@Directive({selector: '[orderCheck0]'}) @Directive({selector: '[orderCheck0]'})
class OrderCheckDirective0 { class OrderCheckDirective0 {
private _name: string; private _name: string;

View File

@ -384,6 +384,20 @@ export function main() {
expect(created).toBe(true); expect(created).toBe(true);
})); }));
it('should instantiate providers with a lifecycle hook eagerly', fakeAsync(() => {
var created = false;
class SomeInjectable {
constructor() { created = true; }
ngOnDestroy() {}
}
var el = createComp(
'<div simpleDirective></div>',
tcb.overrideProviders(SimpleDirective, [SomeInjectable]));
expect(created).toBe(true);
}));
it('should instantiate view providers lazily', fakeAsync(() => { it('should instantiate view providers lazily', fakeAsync(() => {
var created = false; var created = false;
var el = createComp( var el = createComp(