fix(ivy): init hooks should be called once and only once (angular#28239)

marclaval · jasonaden · commit d83307adab10 · 2019-01-25T14:31:23.000-08:00
PR Close angular#28239
diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts
@@ -256,6 +256,7 @@ export function defineComponent<T>(componentDefinition: {
     inputs: null !,   // assigned in noSideEffects
     outputs: null !,  // assigned in noSideEffects
     exportAs: componentDefinition.exportAs || null,
+    onChanges: null,
     onInit: typePrototype.ngOnInit || null,
     doCheck: typePrototype.ngDoCheck || null,
     afterContentInit: typePrototype.ngAfterContentInit || null,
diff --git a/packages/core/src/render3/features/ng_onchanges_feature.ts b/packages/core/src/render3/features/ng_onchanges_feature.ts
@@ -49,16 +49,11 @@ export function NgOnChangesFeature<T>(): DirectiveDefFeature {
 function NgOnChangesFeatureImpl<T>(definition: DirectiveDef<T>): void {
   if (definition.type.prototype.ngOnChanges) {
     definition.setInput = ngOnChangesSetInput;
-
-    const prevDoCheck = definition.doCheck;
-    const prevOnInit = definition.onInit;
-
-    definition.onInit = wrapOnChanges(prevOnInit);
-    definition.doCheck = wrapOnChanges(prevDoCheck);
+    definition.onChanges = wrapOnChanges();
   }
 }
 
-function wrapOnChanges(hook: (() => void) | null) {
+function wrapOnChanges() {
   return function(this: OnChanges) {
     const simpleChangesStore = getSimpleChangesStore(this);
     const current = simpleChangesStore && simpleChangesStore.current;
@@ -68,8 +63,6 @@ function wrapOnChanges(hook: (() => void) | null) {
       simpleChangesStore !.current = null;
       this.ngOnChanges(current);
     }
-
-    hook && hook.call(this);
   };
 }
 
diff --git a/packages/core/src/render3/hooks.ts b/packages/core/src/render3/hooks.ts
@@ -6,12 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {SimpleChanges} from '../interface/simple_change';
 import {assertEqual} from '../util/assert';
 
 import {DirectiveDef} from './interfaces/definition';
 import {TNode} from './interfaces/node';
-import {FLAGS, HookData, LView, LViewFlags, TView} from './interfaces/view';
+import {FLAGS, HookData, InitPhaseState, LView, LViewFlags, TView} from './interfaces/view';
 
 
 
@@ -34,10 +33,15 @@ export function registerPreOrderHooks(
   ngDevMode &&
       assertEqual(tView.firstTemplatePass, true, 'Should only be called on first template pass');
 
-  const {onInit, doCheck} = directiveDef;
+  const {onChanges, onInit, doCheck} = directiveDef;
+
+  if (onChanges) {
+    (tView.initHooks || (tView.initHooks = [])).push(directiveIndex, onChanges);
+    (tView.checkHooks || (tView.checkHooks = [])).push(directiveIndex, onChanges);
+  }
 
   if (onInit) {
-    (tView.initHooks || (tView.initHooks = [])).push(directiveIndex, onInit);
+    (tView.initHooks || (tView.initHooks = [])).push(-directiveIndex, onInit);
   }
 
   if (doCheck) {
@@ -73,7 +77,7 @@ export function registerPostOrderHooks(tView: TView, tNode: TNode): void {
     for (let i = tNode.directiveStart, end = tNode.directiveEnd; i < end; i++) {
       const directiveDef = tView.data[i] as DirectiveDef<any>;
       if (directiveDef.afterContentInit) {
-        (tView.contentHooks || (tView.contentHooks = [])).push(i, directiveDef.afterContentInit);
+        (tView.contentHooks || (tView.contentHooks = [])).push(-i, directiveDef.afterContentInit);
       }
 
       if (directiveDef.afterContentChecked) {
@@ -83,7 +87,7 @@ export function registerPostOrderHooks(tView: TView, tNode: TNode): void {
       }
 
       if (directiveDef.afterViewInit) {
-        (tView.viewHooks || (tView.viewHooks = [])).push(i, directiveDef.afterViewInit);
+        (tView.viewHooks || (tView.viewHooks = [])).push(-i, directiveDef.afterViewInit);
       }
 
       if (directiveDef.afterViewChecked) {
@@ -114,9 +118,10 @@ export function registerPostOrderHooks(tView: TView, tNode: TNode): void {
  */
 export function executeInitHooks(
     currentView: LView, tView: TView, checkNoChangesMode: boolean): void {
-  if (!checkNoChangesMode && currentView[FLAGS] & LViewFlags.RunInit) {
-    executeHooks(currentView, tView.initHooks, tView.checkHooks, checkNoChangesMode);
-    currentView[FLAGS] &= ~LViewFlags.RunInit;
+  if (!checkNoChangesMode) {
+    executeHooks(
+        currentView, tView.initHooks, tView.checkHooks, checkNoChangesMode,
+        InitPhaseState.OnInitHooksToBeRun);
   }
 }
 
@@ -131,12 +136,19 @@ export function executeInitHooks(
  */
 export function executeHooks(
     currentView: LView, firstPassHooks: HookData | null, checkHooks: HookData | null,
-    checkNoChangesMode: boolean): void {
+    checkNoChangesMode: boolean, initPhase: number): void {
   if (checkNoChangesMode) return;
-
-  const hooksToCall = currentView[FLAGS] & LViewFlags.FirstLViewPass ? firstPassHooks : checkHooks;
+  const hooksToCall = (currentView[FLAGS] & LViewFlags.InitPhaseStateMask) === initPhase ?
+      firstPassHooks :
+      checkHooks;
   if (hooksToCall) {
-    callHooks(currentView, hooksToCall);
+    callHooks(currentView, hooksToCall, initPhase);
+  }
+  // The init phase state must be always checked here as it may have been recursively updated
+  if ((currentView[FLAGS] & LViewFlags.InitPhaseStateMask) === initPhase &&
+      initPhase !== InitPhaseState.InitPhaseCompleted) {
+    currentView[FLAGS] &= LViewFlags.IndexWithinInitPhaseReset;
+    currentView[FLAGS] += LViewFlags.InitPhaseStateIncrementer;
   }
 }
 
@@ -147,8 +159,24 @@ export function executeHooks(
  * @param currentView The current view
  * @param arr The array in which the hooks are found
  */
-export function callHooks(currentView: LView, arr: HookData): void {
+export function callHooks(currentView: LView, arr: HookData, initPhase?: number): void {
+  let initHooksCount = 0;
   for (let i = 0; i < arr.length; i += 2) {
-    (arr[i + 1] as() => void).call(currentView[arr[i] as number]);
+    const isInitHook = arr[i] < 0;
+    const directiveIndex = isInitHook ? -arr[i] : arr[i] as number;
+    const directive = currentView[directiveIndex];
+    const hook = arr[i + 1] as() => void;
+    if (isInitHook) {
+      initHooksCount++;
+      const indexWithintInitPhase = currentView[FLAGS] >> LViewFlags.IndexWithinInitPhaseShift;
+      // The init phase state must be always checked here as it may have been recursively updated
+      if (indexWithintInitPhase < initHooksCount &&
+          (currentView[FLAGS] & LViewFlags.InitPhaseStateMask) === initPhase) {
+        currentView[FLAGS] += LViewFlags.IndexWithinInitPhaseIncrementer;
+        hook.call(directive);
+      }
+    } else {
+      hook.call(directive);
+    }
   }
 }
diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts
@@ -32,7 +32,7 @@ import {CssSelectorList, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection'
 import {LQueries} from './interfaces/query';
 import {GlobalTargetResolver, ProceduralRenderer3, RComment, RElement, RText, Renderer3, RendererFactory3, isProceduralRenderer} from './interfaces/renderer';
 import {SanitizerFn} from './interfaces/sanitization';
-import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTENT_QUERIES, CONTEXT, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, INJECTOR, LView, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TAIL, TData, TVIEW, TView} from './interfaces/view';
+import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTENT_QUERIES, CONTEXT, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TAIL, TData, TVIEW, TView} from './interfaces/view';
 import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
 import {appendChild, appendProjectedNode, createTextNode, getLViewChild, insertView, removeView} from './node_manipulation';
 import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher';
@@ -83,7 +83,9 @@ export function refreshDescendantViews(lView: LView) {
     // Content query results must be refreshed before content hooks are called.
     refreshContentQueries(tView);
 
-    executeHooks(lView, tView.contentHooks, tView.contentCheckHooks, checkNoChangesMode);
+    executeHooks(
+        lView, tView.contentHooks, tView.contentCheckHooks, checkNoChangesMode,
+        InitPhaseState.AfterContentInitHooksToBeRun);
 
     setHostBindings(tView, lView);
   }
@@ -159,8 +161,7 @@ export function createLView<T>(
     rendererFactory?: RendererFactory3 | null, renderer?: Renderer3 | null,
     sanitizer?: Sanitizer | null, injector?: Injector | null): LView {
   const lView = tView.blueprint.slice() as LView;
-  lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.RunInit |
-      LViewFlags.FirstLViewPass;
+  lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.FirstLViewPass;
   lView[PARENT] = lView[DECLARATION_VIEW] = parentLView;
   lView[CONTEXT] = context;
   lView[RENDERER_FACTORY] = (rendererFactory || parentLView && parentLView[RENDERER_FACTORY]) !;
diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts
@@ -140,6 +140,7 @@ export interface DirectiveDef<T> extends BaseDef<T> {
   hostBindings: HostBindingsFunction<T>|null;
 
   /* The following are lifecycle hooks for this component */
+  onChanges: (() => void)|null;
   onInit: (() => void)|null;
   doCheck: (() => void)|null;
   afterContentInit: (() => void)|null;
diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts
@@ -215,6 +215,10 @@ export interface LView extends Array<any> {
 
 /** Flags associated with an LView (saved in LView[FLAGS]) */
 export const enum LViewFlags {
+  /** The state of the init phase on the first 2 bits */
+  InitPhaseStateIncrementer = 0b00000000001,
+  InitPhaseStateMask = 0b00000000011,
+
   /**
    * Whether or not the view is in creationMode.
    *
@@ -223,7 +227,7 @@ export const enum LViewFlags {
    * back into the parent view, `data` will be defined and `creationMode` will be
    * improperly reported as false.
    */
-  CreationMode = 0b000000001,
+  CreationMode = 0b00000000100,
 
   /**
    * Whether or not this LView instance is on its first processing pass.
@@ -232,31 +236,43 @@ export const enum LViewFlags {
    * has completed one creation mode run and one update mode run. At this
    * time, the flag is turned off.
    */
-  FirstLViewPass = 0b000000010,
+  FirstLViewPass = 0b00000001000,
 
   /** Whether this view has default change detection strategy (checks always) or onPush */
-  CheckAlways = 0b000000100,
+  CheckAlways = 0b00000010000,
 
   /** Whether or not this view is currently dirty (needing check) */
-  Dirty = 0b000001000,
+  Dirty = 0b00000100000,
 
   /** Whether or not this view is currently attached to change detection tree. */
-  Attached = 0b000010000,
-
-  /**
-   *  Whether or not the init hooks have run.
-   *
-   * If on, the init hooks haven't yet been run and should be executed by the first component that
-   * runs OR the first cR() instruction that runs (so inits are run for the top level view before
-   * any embedded views).
-   */
-  RunInit = 0b000100000,
+  Attached = 0b00001000000,
 
   /** Whether or not this view is destroyed. */
-  Destroyed = 0b001000000,
+  Destroyed = 0b00010000000,
 
   /** Whether or not this view is the root view */
-  IsRoot = 0b010000000,
+  IsRoot = 0b00100000000,
+
+  /**
+   * Index of the current init phase on last 23 bits
+   */
+  IndexWithinInitPhaseIncrementer = 0b01000000000,
+  IndexWithinInitPhaseShift = 9,
+  IndexWithinInitPhaseReset = 0b00111111111,
+}
+
+/**
+ * Possible states of the init phase:
+ * - 00: OnInit hooks to be run.
+ * - 01: AfterContentInit hooks to be run
+ * - 10: AfterViewInit hooks to be run
+ * - 11: All init hooks have been run
+ */
+export const enum InitPhaseState {
+  OnInitHooksToBeRun = 0b00,
+  AfterContentInitHooksToBeRun = 0b01,
+  AfterViewInitHooksToBeRun = 0b10,
+  InitPhaseCompleted = 0b11,
 }
 
 /**
diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts
@@ -11,7 +11,7 @@ import {executeHooks} from './hooks';
 import {ComponentDef, DirectiveDef} from './interfaces/definition';
 import {TElementNode, TNode, TNodeFlags, TViewNode} from './interfaces/node';
 import {LQueries} from './interfaces/query';
-import {BINDING_INDEX, CONTEXT, DECLARATION_VIEW, FLAGS, HOST_NODE, LView, LViewFlags, OpaqueViewState, QUERIES, TVIEW} from './interfaces/view';
+import {BINDING_INDEX, CONTEXT, DECLARATION_VIEW, FLAGS, HOST_NODE, InitPhaseState, LView, LViewFlags, OpaqueViewState, QUERIES, TVIEW} from './interfaces/view';
 import {isContentQueryHost} from './util';
 
 
@@ -336,11 +336,12 @@ export function leaveView(newView: LView): void {
     lView[FLAGS] &= ~LViewFlags.CreationMode;
   } else {
     try {
-      executeHooks(lView, tView.viewHooks, tView.viewCheckHooks, checkNoChangesMode);
+      executeHooks(
+          lView, tView.viewHooks, tView.viewCheckHooks, checkNoChangesMode,
+          InitPhaseState.AfterViewInitHooksToBeRun);
     } finally {
       // Views are clean and in update mode after being checked, so these bits are cleared
       lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass);
-      lView[FLAGS] |= LViewFlags.RunInit;
       lView[BINDING_INDEX] = tView.bindingStartIndex;
     }
   }
diff --git a/packages/core/test/linker/change_detection_integration_spec.ts b/packages/core/test/linker/change_detection_integration_spec.ts
diff --git a/packages/core/test/render3/change_detection_spec.ts b/packages/core/test/render3/change_detection_spec.ts
