fix(upgrade): call ngOnInit() after ngOnChanges() (on components with inputs)

Fixes #18913

packages/upgrade/src/common/downgrade_component_adapter.ts

@@ -99,7 +99,7 @@ export class DowngradeComponentAdapter {
         })(input.prop);
         attrs.$observe(input.attr, observeFn);
 
-        // Use `$watch()` (in addition to `$observe()`) in order to initialize the input  in time
+        // Use `$watch()` (in addition to `$observe()`) in order to initialize the input in time
         // for `ngOnChanges()`. This is necessary if we are already in a `$digest`, which means that
         // `ngOnChanges()` (which is called by a watcher) will run before the `$observe()` callback.
         let unwatch: Function|null = this.componentScope.$watch(() => {
@@ -138,8 +138,7 @@ export class DowngradeComponentAdapter {
         (<OnChanges>this.component).ngOnChanges(inputChanges !);
       }
 
-      // If opted out of propagating digests, invoke change detection
+      // If opted out of propagating digests, invoke change detection when inputs change.
-      // when inputs change
       if (!propagateDigest) {
         detectChanges();
       }
@@ -150,10 +149,16 @@ export class DowngradeComponentAdapter {
       this.componentScope.$watch(this.wrapCallback(detectChanges));
     }
 
-    // Attach the view so that it will be dirty-checked.
+    // If necessary, attach the view so that it will be dirty-checked.
+    // (Allow time for the initial input values to be set and `ngOnChanges()` to be called.)
     if (needsNgZone || !propagateDigest) {
-      const appRef = this.parentInjector.get<ApplicationRef>(ApplicationRef);
+      let unwatch: Function|null = this.componentScope.$watch(() => {
-      appRef.attachView(this.componentRef.hostView);
+        unwatch !();
+        unwatch = null;
+
+        const appRef = this.parentInjector.get<ApplicationRef>(ApplicationRef);
+        appRef.attachView(this.componentRef.hostView);
+      });
     }
   }
 

packages/upgrade/test/static/integration/downgrade_module_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ApplicationRef, Component, Inject, Injector, Input, NgModule, NgZone, OnChanges, OnDestroy, StaticProvider, ViewRef, destroyPlatform} from '@angular/core';
+import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ApplicationRef, Component, DoCheck, Inject, Injector, Input, NgModule, NgZone, OnChanges, OnDestroy, OnInit, StaticProvider, ViewRef, destroyPlatform} from '@angular/core';
 import {async, fakeAsync, tick} from '@angular/core/testing';
 import {BrowserModule} from '@angular/platform-browser';
 import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
@@ -16,7 +16,7 @@ import {$ROOT_SCOPE, INJECTOR_KEY, LAZY_MODULE_REF} from '@angular/upgrade/src/c
 import {LazyModuleRef} from '@angular/upgrade/src/common/util';
 import {downgradeComponent, downgradeModule} from '@angular/upgrade/static';
 
-import {html} from '../test_helpers';
+import {html, multiTrim} from '../test_helpers';
 
 
 export function main() {
@@ -306,6 +306,162 @@ export function main() {
            });
          }));
 
+      it('should run the lifecycle hooks in the correct order', async(() => {
+           const logs: string[] = [];
+           let rootScope: angular.IRootScopeService;
+
+           @Component({
+             selector: 'ng2',
+             template: `
+               {{ value }}
+               <button (click)="value = 'qux'"></button>
+               <ng-content></ng-content>
+             `
+           })
+           class Ng2Component implements AfterContentChecked,
+               AfterContentInit, AfterViewChecked, AfterViewInit, DoCheck, OnChanges, OnDestroy,
+               OnInit {
+             @Input() value = 'foo';
+
+             ngAfterContentChecked() { this.log('AfterContentChecked'); }
+             ngAfterContentInit() { this.log('AfterContentInit'); }
+             ngAfterViewChecked() { this.log('AfterViewChecked'); }
+             ngAfterViewInit() { this.log('AfterViewInit'); }
+             ngDoCheck() { this.log('DoCheck'); }
+             ngOnChanges() { this.log('OnChanges'); }
+             ngOnDestroy() { this.log('OnDestroy'); }
+             ngOnInit() { this.log('OnInit'); }
+
+             private log(hook: string) { logs.push(`${hook}(${this.value})`); }
+           }
+
+           @NgModule({
+             declarations: [Ng2Component],
+             entryComponents: [Ng2Component],
+             imports: [BrowserModule],
+           })
+           class Ng2Module {
+             ngDoBootstrap() {}
+           }
+
+           const bootstrapFn = (extraProviders: StaticProvider[]) =>
+               platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module);
+           const lazyModuleName = downgradeModule<Ng2Module>(bootstrapFn);
+           const ng1Module =
+               angular.module('ng1', [lazyModuleName])
+                   .directive('ng2', downgradeComponent({component: Ng2Component, propagateDigest}))
+                   .run(($rootScope: angular.IRootScopeService) => {
+                     rootScope = $rootScope;
+                     rootScope.value = 'bar';
+                   });
+
+           const element =
+               html('<div><ng2 value="{{ value }}" ng-if="!hideNg2">Content</ng2></div>');
+           angular.bootstrap(element, [ng1Module.name]);
+
+           setTimeout(() => {    // Wait for the module to be bootstrapped.
+             setTimeout(() => {  // Wait for `$evalAsync()` to propagate inputs.
+               const button = element.querySelector('button') !;
+
+               // Once initialized.
+               expect(multiTrim(element.textContent)).toBe('bar Content');
+               expect(logs).toEqual([
+                 // `ngOnChanges()` call triggered directly through the `inputChanges` $watcher.
+                 'OnChanges(bar)',
+                 // Initial CD triggered directly through the `detectChanges()` or `inputChanges`
+                 // $watcher (for `propagateDigest` true/false respectively).
+                 'OnInit(bar)',
+                 'DoCheck(bar)',
+                 'AfterContentInit(bar)',
+                 'AfterContentChecked(bar)',
+                 'AfterViewInit(bar)',
+                 'AfterViewChecked(bar)',
+                 ...(propagateDigest ?
+                         [
+                           // CD triggered directly through the `detectChanges()` $watcher (2nd
+                           // $digest).
+                           'DoCheck(bar)',
+                           'AfterContentChecked(bar)',
+                           'AfterViewChecked(bar)',
+                         ] :
+                         []),
+                 // CD triggered due to entering/leaving the NgZone (in `downgradeFn()`).
+                 'DoCheck(bar)',
+                 'AfterContentChecked(bar)',
+                 'AfterViewChecked(bar)',
+               ]);
+               logs.length = 0;
+
+               // Change inputs and run `$digest`.
+               rootScope.$apply('value = "baz"');
+               expect(multiTrim(element.textContent)).toBe('baz Content');
+               expect(logs).toEqual([
+                 // `ngOnChanges()` call triggered directly through the `inputChanges` $watcher.
+                 'OnChanges(baz)',
+                 // `propagateDigest: true` (3 CD runs):
+                 //   - CD triggered due to entering/leaving the NgZone (in `inputChanges`
+                 //   $watcher).
+                 //   - CD triggered directly through the `detectChanges()` $watcher.
+                 //   - CD triggered due to entering/leaving the NgZone (in `detectChanges`
+                 //   $watcher).
+                 // `propagateDigest: false` (2 CD runs):
+                 //   - CD triggered directly through the `inputChanges` $watcher.
+                 //   - CD triggered due to entering/leaving the NgZone (in `inputChanges`
+                 //   $watcher).
+                 'DoCheck(baz)',
+                 'AfterContentChecked(baz)',
+                 'AfterViewChecked(baz)',
+                 'DoCheck(baz)',
+                 'AfterContentChecked(baz)',
+                 'AfterViewChecked(baz)',
+                 ...(propagateDigest ?
+                         [
+                           'DoCheck(baz)',
+                           'AfterContentChecked(baz)',
+                           'AfterViewChecked(baz)',
+                         ] :
+                         []),
+               ]);
+               logs.length = 0;
+
+               // Run `$digest` (without changing inputs).
+               rootScope.$digest();
+               expect(multiTrim(element.textContent)).toBe('baz Content');
+               expect(logs).toEqual(
+                   propagateDigest ?
+                       [
+                         // CD triggered directly through the `detectChanges()` $watcher.
+                         'DoCheck(baz)',
+                         'AfterContentChecked(baz)',
+                         'AfterViewChecked(baz)',
+                         // CD triggered due to entering/leaving the NgZone (in the above $watcher).
+                         'DoCheck(baz)',
+                         'AfterContentChecked(baz)',
+                         'AfterViewChecked(baz)',
+                       ] :
+                       []);
+               logs.length = 0;
+
+               // Trigger change detection (without changing inputs).
+               button.click();
+               expect(multiTrim(element.textContent)).toBe('qux Content');
+               expect(logs).toEqual([
+                 'DoCheck(qux)',
+                 'AfterContentChecked(qux)',
+                 'AfterViewChecked(qux)',
+               ]);
+               logs.length = 0;
+
+               // Destroy the component.
+               rootScope.$apply('hideNg2 = true');
+               expect(logs).toEqual([
+                 'OnDestroy(qux)',
+               ]);
+               logs.length = 0;
+             });
+           });
+         }));
+
       it('should detach hostViews from the ApplicationRef once destroyed', async(() => {
            let ng2Component: Ng2Component;
 
@@ -339,17 +495,18 @@ export function main() {
            const $injector = angular.bootstrap(element, [ng1Module.name]);
            const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService;
 
-           // Wait for the module to be bootstrapped.
+           setTimeout(() => {    // Wait for the module to be bootstrapped.
-           setTimeout(() => {
+             setTimeout(() => {  // Wait for the hostView to be attached (during the `$digest`).
-             const hostView: ViewRef =
+               const hostView: ViewRef =
-                 (ng2Component.appRef.attachView as jasmine.Spy).calls.mostRecent().args[0];
+                   (ng2Component.appRef.attachView as jasmine.Spy).calls.mostRecent().args[0];
 
-             expect(hostView.destroyed).toBe(false);
+               expect(hostView.destroyed).toBe(false);
 
-             $rootScope.$apply('hideNg2 = true');
+               $rootScope.$apply('hideNg2 = true');
 
-             expect(hostView.destroyed).toBe(true);
+               expect(hostView.destroyed).toBe(true);
-             expect(ng2Component.appRef.detachView).toHaveBeenCalledWith(hostView);
+               expect(ng2Component.appRef.detachView).toHaveBeenCalledWith(hostView);
+             });
            });
          }));