diff --git a/packages/core/src/view/provider.ts b/packages/core/src/view/provider.ts index f8ae12049c..1576b566b5 100644 --- a/packages/core/src/view/provider.ts +++ b/packages/core/src/view/provider.ts @@ -14,7 +14,7 @@ import {ViewContainerRef} from '../linker/view_container_ref'; import {Renderer as RendererV1, Renderer2} from '../render/api'; import {createChangeDetectorRef, createInjector, createRendererV1} from './refs'; -import {BindingDef, BindingFlags, DepDef, DepFlags, NodeDef, NodeFlags, OutputDef, OutputType, ProviderData, QueryValueType, Services, ViewData, ViewFlags, ViewState, asElementData, asProviderData} from './types'; +import {BindingDef, BindingFlags, DepDef, DepFlags, NodeDef, NodeFlags, OutputDef, OutputType, ProviderData, QueryValueType, Services, ViewData, ViewFlags, ViewState, asElementData, asProviderData, shouldCallLifecycleInitHook} from './types'; import {calcBindingFlags, checkBinding, dispatchEvent, isComponentView, splitDepsDsl, splitMatchedQueriesDsl, tokenKey, viewParentEl} from './util'; const RendererV1TokenKey = tokenKey(RendererV1); @@ -198,7 +198,8 @@ export function checkAndUpdateDirectiveInline( if (changes) { directive.ngOnChanges(changes); } - if ((view.state & ViewState.FirstCheck) && (def.flags & NodeFlags.OnInit)) { + if ((def.flags & NodeFlags.OnInit) && + shouldCallLifecycleInitHook(view, ViewState.InitState_CallingOnInit, def.nodeIndex)) { directive.ngOnInit(); } if (def.flags & NodeFlags.DoCheck) { @@ -222,7 +223,8 @@ export function checkAndUpdateDirectiveDynamic( if (changes) { directive.ngOnChanges(changes); } - if ((view.state & ViewState.FirstCheck) && (def.flags & NodeFlags.OnInit)) { + if ((def.flags & NodeFlags.OnInit) && + shouldCallLifecycleInitHook(view, ViewState.InitState_CallingOnInit, def.nodeIndex)) { directive.ngOnInit(); } if (def.flags & NodeFlags.DoCheck) { @@ -447,17 +449,61 @@ function updateProp( return changes; } +// This function calls the ngAfterContentCheck, ngAfterContentInit, +// ngAfterViewCheck, and ngAfterViewInit lifecycle hooks (depending on the node +// flags in lifecycle). Unlike ngDoCheck, ngOnChanges and ngOnInit, which are +// called during a pre-order traversal of the view tree (that is calling the +// parent hooks before the child hooks) these events are sent in using a +// post-order traversal of the tree (children before parents). This changes the +// meaning of initIndex in the view state. For ngOnInit, initIndex tracks the +// expected nodeIndex which a ngOnInit should be called. When sending +// ngAfterContentInit and ngAfterViewInit it is the expected count of +// ngAfterContentInit or ngAfterViewInit methods that have been called. This +// ensure that dispite being called recursively or after picking up after an +// exception, the ngAfterContentInit or ngAfterViewInit will be called on the +// correct nodes. Consider for example, the following (where E is an element +// and D is a directive) +// Tree: pre-order index post-order index +// E1 0 6 +// E2 1 1 +// D3 2 0 +// E4 3 5 +// E5 4 4 +// E6 5 2 +// E7 6 3 +// As can be seen, the post-order index has an unclear relationship to the +// pre-order index (postOrderIndex === preOrderIndex - parentCount + +// childCount). Since number of calls to ngAfterContentInit and ngAfterViewInit +// are stable (will be the same for the same view regardless of exceptions or +// recursion) we just need to count them which will roughly correspond to the +// post-order index (it skips elements and directives that do not have +// lifecycle hooks). +// +// For example, if an exception is raised in the E6.onAfterViewInit() the +// initIndex is left at 3 (by shouldCallLifecycleInitHook() which set it to +// initIndex + 1). When checkAndUpdateView() is called again D3, E2 and E6 will +// not have their ngAfterViewInit() called but, starting with E7, the rest of +// the view will begin getting ngAfterViewInit() called until a check and +// pass is complete. +// +// This algorthim also handles recursion. Consider if E4's ngAfterViewInit() +// indirectly calls E1's ChangeDetectorRef.detectChanges(). The expected +// initIndex is set to 6, the recusive checkAndUpdateView() starts walk again. +// D3, E2, E6, E7, E5 and E4 are skipped, ngAfterViewInit() is called on E1. +// When the recursion returns the initIndex will be 7 so E1 is skipped as it +// has already been called in the recursively called checkAnUpdateView(). export function callLifecycleHooksChildrenFirst(view: ViewData, lifecycles: NodeFlags) { if (!(view.def.nodeFlags & lifecycles)) { return; } const nodes = view.def.nodes; + let initIndex = 0; for (let i = 0; i < nodes.length; i++) { const nodeDef = nodes[i]; let parent = nodeDef.parent; if (!parent && nodeDef.flags & lifecycles) { // matching root node (e.g. a pipe) - callProviderLifecycles(view, i, nodeDef.flags & lifecycles); + callProviderLifecycles(view, i, nodeDef.flags & lifecycles, initIndex++); } if ((nodeDef.childFlags & lifecycles) === 0) { // no child matches one of the lifecycles @@ -467,25 +513,28 @@ export function callLifecycleHooksChildrenFirst(view: ViewData, lifecycles: Node i === parent.nodeIndex + parent.childCount) { // last child of an element if (parent.directChildFlags & lifecycles) { - callElementProvidersLifecycles(view, parent, lifecycles); + initIndex = callElementProvidersLifecycles(view, parent, lifecycles, initIndex); } parent = parent.parent; } } } -function callElementProvidersLifecycles(view: ViewData, elDef: NodeDef, lifecycles: NodeFlags) { +function callElementProvidersLifecycles( + view: ViewData, elDef: NodeDef, lifecycles: NodeFlags, initIndex: number): number { for (let i = elDef.nodeIndex + 1; i <= elDef.nodeIndex + elDef.childCount; i++) { const nodeDef = view.def.nodes[i]; if (nodeDef.flags & lifecycles) { - callProviderLifecycles(view, i, nodeDef.flags & lifecycles); + callProviderLifecycles(view, i, nodeDef.flags & lifecycles, initIndex++); } // only visit direct children i += nodeDef.childCount; } + return initIndex; } -function callProviderLifecycles(view: ViewData, index: number, lifecycles: NodeFlags) { +function callProviderLifecycles( + view: ViewData, index: number, lifecycles: NodeFlags, initIndex: number) { const providerData = asProviderData(view, index); if (!providerData) { return; @@ -495,13 +544,15 @@ function callProviderLifecycles(view: ViewData, index: number, lifecycles: NodeF return; } Services.setCurrentNode(view, index); - if (lifecycles & NodeFlags.AfterContentInit) { + if (lifecycles & NodeFlags.AfterContentInit && + shouldCallLifecycleInitHook(view, ViewState.InitState_CallingAfterContentInit, initIndex)) { provider.ngAfterContentInit(); } if (lifecycles & NodeFlags.AfterContentChecked) { provider.ngAfterContentChecked(); } - if (lifecycles & NodeFlags.AfterViewInit) { + if (lifecycles & NodeFlags.AfterViewInit && + shouldCallLifecycleInitHook(view, ViewState.InitState_CallingAfterViewInit, initIndex)) { provider.ngAfterViewInit(); } if (lifecycles & NodeFlags.AfterViewChecked) { diff --git a/packages/core/src/view/types.ts b/packages/core/src/view/types.ts index 1340815c44..dd6cbeaacb 100644 --- a/packages/core/src/view/types.ts +++ b/packages/core/src/view/types.ts @@ -354,6 +354,7 @@ export interface ViewData { state: ViewState; oldValues: any[]; disposables: DisposableFn[]|null; + initIndex: number; } /** @@ -369,8 +370,52 @@ export const enum ViewState { CheckProjectedViews = 1 << 6, Destroyed = 1 << 7, + // InitState Uses 3 bits + InitState_Mask = 7 << 8, + InitState_BeforeInit = 0 << 8, + InitState_CallingOnInit = 1 << 8, + InitState_CallingAfterContentInit = 2 << 8, + InitState_CallingAfterViewInit = 3 << 8, + InitState_AfterInit = 4 << 8, + CatDetectChanges = Attached | ChecksEnabled, - CatInit = BeforeFirstCheck | CatDetectChanges + CatInit = BeforeFirstCheck | CatDetectChanges | InitState_BeforeInit +} + +// Called before each cycle of a view's check to detect whether this is in the +// initState for which we need to call ngOnInit, ngAfterContentInit or ngAfterViewInit +// lifecycle methods. Returns true if this check cycle should call lifecycle +// methods. +export function shiftInitState( + view: ViewData, priorInitState: ViewState, newInitState: ViewState): boolean { + // Only update the InitState if we are currently in the prior state. + // For example, only move into CallingInit if we are in BeforeInit. Only + // move into CallingContentInit if we are in CallingInit. Normally this will + // always be true because of how checkCycle is called in checkAndUpdateView. + // However, if checkAndUpdateView is called recursively or if an exception is + // thrown while checkAndUpdateView is running, checkAndUpdateView starts over + // from the beginning. This ensures the state is monotonically increasing, + // terminating in the AfterInit state, which ensures the Init methods are called + // at least once and only once. + const state = view.state; + const initState = state & ViewState.InitState_Mask; + if (initState === priorInitState) { + view.state = (state & ~ViewState.InitState_Mask) | newInitState; + view.initIndex = -1; + return true; + } + return initState === newInitState; +} + +// Returns true if the lifecycle init method should be called for the node with +// the given init index. +export function shouldCallLifecycleInitHook( + view: ViewData, initState: ViewState, index: number): boolean { + if ((view.state & ViewState.InitState_Mask) === initState && view.initIndex <= index) { + view.initIndex = index + 1; + return true; + } + return false; } export interface DisposableFn { (): void; } diff --git a/packages/core/src/view/view.ts b/packages/core/src/view/view.ts index d37a3106b1..434473f435 100644 --- a/packages/core/src/view/view.ts +++ b/packages/core/src/view/view.ts @@ -16,7 +16,7 @@ import {checkAndUpdatePureExpressionDynamic, checkAndUpdatePureExpressionInline, import {checkAndUpdateQuery, createQuery} from './query'; import {createTemplateData, createViewContainerData} from './refs'; import {checkAndUpdateTextDynamic, checkAndUpdateTextInline, createText} from './text'; -import {ArgumentType, CheckType, ElementData, NodeData, NodeDef, NodeFlags, ProviderData, RootData, Services, ViewData, ViewDefinition, ViewFlags, ViewHandleEventFn, ViewState, ViewUpdateFn, asElementData, asQueryList, asTextData} from './types'; +import {ArgumentType, CheckType, ElementData, NodeData, NodeDef, NodeFlags, ProviderData, RootData, Services, ViewData, ViewDefinition, ViewFlags, ViewHandleEventFn, ViewState, ViewUpdateFn, asElementData, asQueryList, asTextData, shiftInitState} from './types'; import {NOOP, checkBindingNoChanges, isComponentView, markParentViewsForCheckProjectedViews, resolveDefinition, tokenKey} from './util'; import {detachProjectedView} from './view_attach'; @@ -236,7 +236,8 @@ function createView( context: null, component: null, nodes, state: ViewState.CatInit, root, renderer, - oldValues: new Array(def.bindingCount), disposables + oldValues: new Array(def.bindingCount), disposables, + initIndex: -1 }; return view; } @@ -353,29 +354,32 @@ export function checkAndUpdateView(view: ViewData) { } else { view.state &= ~ViewState.FirstCheck; } + shiftInitState(view, ViewState.InitState_BeforeInit, ViewState.InitState_CallingOnInit); markProjectedViewsForCheck(view); Services.updateDirectives(view, CheckType.CheckAndUpdate); execEmbeddedViewsAction(view, ViewAction.CheckAndUpdate); execQueriesAction( view, NodeFlags.TypeContentQuery, NodeFlags.DynamicQuery, CheckType.CheckAndUpdate); - + let callInit = shiftInitState( + view, ViewState.InitState_CallingOnInit, ViewState.InitState_CallingAfterContentInit); callLifecycleHooksChildrenFirst( - view, NodeFlags.AfterContentChecked | - (view.state & ViewState.FirstCheck ? NodeFlags.AfterContentInit : 0)); + view, NodeFlags.AfterContentChecked | (callInit ? NodeFlags.AfterContentInit : 0)); Services.updateRenderer(view, CheckType.CheckAndUpdate); execComponentViewsAction(view, ViewAction.CheckAndUpdate); execQueriesAction( view, NodeFlags.TypeViewQuery, NodeFlags.DynamicQuery, CheckType.CheckAndUpdate); + callInit = shiftInitState( + view, ViewState.InitState_CallingAfterContentInit, ViewState.InitState_CallingAfterViewInit); callLifecycleHooksChildrenFirst( - view, NodeFlags.AfterViewChecked | - (view.state & ViewState.FirstCheck ? NodeFlags.AfterViewInit : 0)); + view, NodeFlags.AfterViewChecked | (callInit ? NodeFlags.AfterViewInit : 0)); if (view.def.flags & ViewFlags.OnPush) { view.state &= ~ViewState.ChecksEnabled; } view.state &= ~(ViewState.CheckProjectedViews | ViewState.CheckProjectedView); + shiftInitState(view, ViewState.InitState_CallingAfterViewInit, ViewState.InitState_AfterInit); } export function checkAndUpdateNode( diff --git a/packages/core/test/linker/change_detection_integration_spec.ts b/packages/core/test/linker/change_detection_integration_spec.ts index 1fb3c8e435..110bab84c1 100644 --- a/packages/core/test/linker/change_detection_integration_spec.ts +++ b/packages/core/test/linker/change_detection_integration_spec.ts @@ -1152,7 +1152,6 @@ export function main() { ]); })); }); - }); describe('enforce no new changes', () => { @@ -1463,6 +1462,151 @@ export function main() { expect(divEl.nativeElement).toHaveCssClass('foo'); }); }); + + describe('lifecycle asserts', () => { + let logged: string[]; + + function log(value: string) { logged.push(value); } + function clearLog() { logged = []; } + + function expectOnceAndOnlyOnce(log: string) { + expect(logged.indexOf(log) >= 0) + .toBeTruthy(`'${log}' not logged. Log was ${JSON.stringify(logged)}`); + expect(logged.lastIndexOf(log) === logged.indexOf(log)) + .toBeTruthy(`'${log}' logged more than once. Log was ${JSON.stringify(logged)}`); + } + + beforeEach(() => { clearLog(); }); + + enum LifetimeMethods { + None = 0, + ngOnInit = 1 << 0, + ngOnChanges = 1 << 1, + ngAfterViewInit = 1 << 2, + ngAfterContentInit = 1 << 3, + ngDoCheck = 1 << 4, + InitMethods = ngOnInit | ngAfterViewInit | ngAfterContentInit, + InitMethodsAndChanges = InitMethods | ngOnChanges, + All = InitMethodsAndChanges | ngDoCheck, + } + + function forEachMethod(methods: LifetimeMethods, cb: (method: LifetimeMethods) => void) { + if (methods & LifetimeMethods.ngOnInit) cb(LifetimeMethods.ngOnInit); + if (methods & LifetimeMethods.ngOnChanges) cb(LifetimeMethods.ngOnChanges); + if (methods & LifetimeMethods.ngAfterContentInit) cb(LifetimeMethods.ngAfterContentInit); + if (methods & LifetimeMethods.ngAfterViewInit) cb(LifetimeMethods.ngAfterViewInit); + if (methods & LifetimeMethods.ngDoCheck) cb(LifetimeMethods.ngDoCheck); + } + + interface Options { + childRecursion: LifetimeMethods; + childThrows: LifetimeMethods; + } + + describe('calling init', () => { + function initialize(options: Options) { + @Component({selector: 'my-child', template: ''}) + class MyChild { + private thrown = LifetimeMethods.None; + + @Input() inp: boolean; + @Output() outp = new EventEmitter(); + + constructor() {} + + ngDoCheck() { this.check(LifetimeMethods.ngDoCheck); } + ngOnInit() { this.check(LifetimeMethods.ngOnInit); } + ngOnChanges() { this.check(LifetimeMethods.ngOnChanges); } + ngAfterViewInit() { this.check(LifetimeMethods.ngAfterViewInit); } + ngAfterContentInit() { this.check(LifetimeMethods.ngAfterContentInit); } + + private check(method: LifetimeMethods) { + log(`MyChild::${LifetimeMethods[method]}()`); + + if ((options.childRecursion & method) !== 0) { + if (logged.length < 20) { + this.outp.emit(null); + } else { + fail(`Unexpected MyChild::${LifetimeMethods[method]} recursion`); + } + } + if ((options.childThrows & method) !== 0) { + if ((this.thrown & method) === 0) { + this.thrown |= method; + log(`()`); + throw new Error(`Throw from MyChild::${LifetimeMethods[method]}`); + } + } + } + } + + @Component({ + selector: 'my-component', + template: `` + }) + class MyComponent { + constructor(private changeDetectionRef: ChangeDetectorRef) {} + ngDoCheck() { this.check(LifetimeMethods.ngDoCheck); } + ngOnInit() { this.check(LifetimeMethods.ngOnInit); } + ngAfterViewInit() { this.check(LifetimeMethods.ngAfterViewInit); } + ngAfterContentInit() { this.check(LifetimeMethods.ngAfterContentInit); } + onOutp() { + log(''); + this.changeDetectionRef.detectChanges(); + log(''); + } + + private check(method: LifetimeMethods) { + log(`MyComponent::${LifetimeMethods[method]}()`); + } + } + + TestBed.configureTestingModule({declarations: [MyChild, MyComponent]}); + + return createCompFixture(``); + } + + function ensureOneInit(options: Options) { + const ctx = initialize(options); + + + const throws = options.childThrows != LifetimeMethods.None; + if (throws) { + log(``); + expect(() => { + // Expect child to throw. + ctx.detectChanges(); + }).toThrow(); + log(``); + log(``); + } + ctx.detectChanges(); + if (throws) log(``); + expectOnceAndOnlyOnce('MyComponent::ngOnInit()'); + expectOnceAndOnlyOnce('MyChild::ngOnInit()'); + expectOnceAndOnlyOnce('MyComponent::ngAfterViewInit()'); + expectOnceAndOnlyOnce('MyComponent::ngAfterContentInit()'); + expectOnceAndOnlyOnce('MyChild::ngAfterViewInit()'); + expectOnceAndOnlyOnce('MyChild::ngAfterContentInit()'); + } + + forEachMethod(LifetimeMethods.InitMethodsAndChanges, method => { + it(`should ensure that init hooks are called once an only once with recursion in ${LifetimeMethods[method]} `, + () => { + // Ensure all the init methods are called once. + ensureOneInit({childRecursion: method, childThrows: LifetimeMethods.None}); + }); + }); + forEachMethod(LifetimeMethods.All, method => { + it(`should ensure that init hooks are called once an only once with a throw in ${LifetimeMethods[method]} `, + () => { + // Ensure all the init methods are called once. + // the first cycle throws but the next cycle should complete the inits. + ensureOneInit({childRecursion: LifetimeMethods.None, childThrows: method}); + }); + }); + }); + }); }); }