fix(core): ensure init lifecycle events are called (#20258)

Throwing an exception in a lifecycle event will delay but not
prevent an Init method, such as `ngOnInit`, `ngAfterContentInit`,
or `ngAfterViewInit`, from being called. Also, calling `detectChanges()`
in a way that causes duplicate change detection (such as a
child component causing a parent to call `detectChanges()` on its
own `ChangeDetectorRef`, will no longer prevent change `ngOnInit`,
`ngAfterContentInit` and `ngAfterViewInit` from being called.

With this change lifecycle methods are still not guarenteed to be
called but the Init methods will be called if at least one change
detection pass on its view is completed.

Fixes: #17035

PR Close #20258
This commit is contained in:
Chuck Jazdzewski 2017-11-07 16:44:32 -08:00 committed by Jason Aden
parent 743651f5e8
commit 24cf8b3269
4 changed files with 263 additions and 19 deletions

View File

@ -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) {

View File

@ -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; }

View File

@ -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(

View File

@ -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<any>();
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 from MyChild::${LifetimeMethods[method]}>()`);
throw new Error(`Throw from MyChild::${LifetimeMethods[method]}`);
}
}
}
}
@Component({
selector: 'my-component',
template: `<my-child [inp]='true' (outp)='onOutp()'></my-child>`
})
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('<RECURSION START>');
this.changeDetectionRef.detectChanges();
log('<RECURSION DONE>');
}
private check(method: LifetimeMethods) {
log(`MyComponent::${LifetimeMethods[method]}()`);
}
}
TestBed.configureTestingModule({declarations: [MyChild, MyComponent]});
return createCompFixture(`<my-component></my-component>`);
}
function ensureOneInit(options: Options) {
const ctx = initialize(options);
const throws = options.childThrows != LifetimeMethods.None;
if (throws) {
log(`<CYCLE 0 START>`);
expect(() => {
// Expect child to throw.
ctx.detectChanges();
}).toThrow();
log(`<CYCLE 0 END>`);
log(`<CYCLE 1 START>`);
}
ctx.detectChanges();
if (throws) log(`<CYCLE 1 DONE>`);
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});
});
});
});
});
});
}