fix(upgrade): initialize all inputs in time for `ngOnChanges()`

Previously, non-bracketed inputs (e.g. `xyz="foo"`) on downgraded components
were initialized using `attrs.$observe()` (which uses `$evalAsync()` under the
hood), while bracketed inputs (e.g. `[xyz]="'foo'"`) were initialized using
`$watch()`. If the downgraded component was created during a `$digest` (e.g. by
an `ng-if` watcher), the non-bracketed inputs were not initialized in time for
the initial call to `ngOnChanges()` and `ngOnInit()`.

This commit fixes it by using `$watch()` to initialize all inputs. `$observe()`
is still used for subsequent updates on non-bracketed inputs, because it is more
performant.

Fixes #16212

packages/upgrade/src/common/downgrade_component_adapter.ts

@@ -11,7 +11,7 @@ import {ChangeDetectorRef, ComponentFactory, ComponentRef, EventEmitter, Injecto
 import * as angular from './angular1';
 import {PropertyBinding} from './component_info';
 import {$SCOPE} from './constants';
-import {getAttributesAsArray, getComponentName, hookupNgModel} from './util';
+import {getAttributesAsArray, getComponentName, hookupNgModel, strictEquals} from './util';
 
 const INITIAL_VALUE = {
   __UNINITIALIZED__: true
@@ -75,16 +75,28 @@ export class DowngradeComponentAdapter {
         const observeFn = (prop => {
           let prevValue = INITIAL_VALUE;
           return (currValue: any) => {
-            if (prevValue === INITIAL_VALUE) {
+            // Initially, both `$observe()` and `$watch()` will call this function.
+            if (!strictEquals(prevValue, currValue)) {
+              if (prevValue === INITIAL_VALUE) {
+                prevValue = currValue;
+              }
+
+              this.updateInput(prop, prevValue, currValue);
               prevValue = currValue;
             }
-
-            this.updateInput(prop, prevValue, currValue);
-            prevValue = currValue;
           };
         })(input.prop);
         attrs.$observe(input.attr, observeFn);
 
+        // 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: any = this.componentScope.$watch(() => {
+          unwatch();
+          unwatch = null;
+          observeFn((attrs as any)[input.attr]);
+        });
+
       } else if (attrs.hasOwnProperty(input.bindAttr)) {
         expr = (attrs as any /** TODO #9100 */)[input.bindAttr];
       } else if (attrs.hasOwnProperty(input.bracketAttr)) {

packages/upgrade/src/common/util.ts

@@ -75,3 +75,10 @@ export function hookupNgModel(ngModel: angular.INgModelController, component: an
     component.registerOnChange(ngModel.$setViewValue.bind(ngModel));
   }
 }
+
+/**
+ * Test two values for strict equality, accounting for the fact that `NaN !== NaN`.
+ */
+export function strictEquals(val1: any, val2: any): boolean {
+  return val1 === val2 || (val1 !== val1 && val2 !== val2);
+}

packages/upgrade/src/dynamic/upgrade_ng1_adapter.ts

@@ -10,7 +10,7 @@ import {Directive, DoCheck, ElementRef, EventEmitter, Inject, OnChanges, OnInit,
 
 import * as angular from '../common/angular1';
 import {$COMPILE, $CONTROLLER, $HTTP_BACKEND, $SCOPE, $TEMPLATE_CACHE} from '../common/constants';
-import {controllerKey} from '../common/util';
+import {controllerKey, strictEquals} from '../common/util';
 
 
 interface IBindingDestination {
@@ -309,8 +309,7 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
     checkProperties.forEach((propName, i) => {
       const value = destinationObj ![propName];
       const last = lastValues[i];
-      if (value !== last &&
-          (value === value || last === last)) {  // ignore NaN values (NaN !== NaN)
+      if (!strictEquals(last, value)) {
         const eventEmitter: EventEmitter<any> = (this as any)[propOuts[i]];
         eventEmitter.emit(lastValues[i] = value);
       }

packages/upgrade/test/dynamic/upgrade_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ChangeDetectorRef, Class, Component, EventEmitter, Input, NO_ERRORS_SCHEMA, NgModule, NgZone, SimpleChange, SimpleChanges, Testability, destroyPlatform, forwardRef} from '@angular/core';
+import {ChangeDetectorRef, Class, Component, EventEmitter, Input, NO_ERRORS_SCHEMA, NgModule, NgZone, OnChanges, SimpleChange, SimpleChanges, Testability, destroyPlatform, forwardRef} from '@angular/core';
 import {async, fakeAsync, flushMicrotasks, tick} from '@angular/core/testing';
 import {BrowserModule} from '@angular/platform-browser';
 import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
@@ -410,6 +410,63 @@ export function main() {
 
          }));
 
+      it('should initialize inputs in time for `ngOnChanges`', async(() => {
+           const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
+
+           @Component({
+             selector: 'ng2',
+             template: `
+               ngOnChangesCount: {{ ngOnChangesCount }} |
+               firstChangesCount: {{ firstChangesCount }} |
+               initialValue: {{ initialValue }}`
+           })
+           class Ng2Component implements OnChanges {
+             ngOnChangesCount = 0;
+             firstChangesCount = 0;
+             initialValue: string;
+             @Input() foo: string;
+
+             ngOnChanges(changes: SimpleChanges) {
+               this.ngOnChangesCount++;
+
+               if (this.ngOnChangesCount === 1) {
+                 this.initialValue = this.foo;
+               }
+
+               if (changes['foo'] && changes['foo'].isFirstChange()) {
+                 this.firstChangesCount++;
+               }
+             }
+           }
+
+           @NgModule({imports: [BrowserModule], declarations: [Ng2Component]})
+           class Ng2Module {
+           }
+
+           const ng1Module = angular.module('ng1', []).directive(
+               'ng2', adapter.downgradeNg2Component(Ng2Component));
+
+           const element = html(`
+             <ng2 [foo]="'foo'"></ng2>
+             <ng2 foo="bar"></ng2>
+             <ng2 [foo]="'baz'" ng-if="true"></ng2>
+             <ng2 foo="qux" ng-if="true"></ng2>
+           `);
+
+           adapter.bootstrap(element, ['ng1']).ready(ref => {
+             const nodes = element.querySelectorAll('ng2');
+             const expectedTextWith = (value: string) =>
+                 `ngOnChangesCount: 1 | firstChangesCount: 1 | initialValue: ${value}`;
+
+             expect(multiTrim(nodes[0].textContent)).toBe(expectedTextWith('foo'));
+             expect(multiTrim(nodes[1].textContent)).toBe(expectedTextWith('bar'));
+             expect(multiTrim(nodes[2].textContent)).toBe(expectedTextWith('baz'));
+             expect(multiTrim(nodes[3].textContent)).toBe(expectedTextWith('qux'));
+
+             ref.dispose();
+           });
+         }));
+
       it('should bind to ng-model', async(() => {
            const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
            const ng1Module = angular.module('ng1', []);

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

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Compiler, Component, ComponentFactoryResolver, EventEmitter, Injector, NgModule, NgModuleRef, OnChanges, OnDestroy, SimpleChanges, destroyPlatform} from '@angular/core';
+import {Compiler, Component, ComponentFactoryResolver, EventEmitter, Injector, Input, NgModule, NgModuleRef, OnChanges, OnDestroy, SimpleChanges, destroyPlatform} from '@angular/core';
 import {async} from '@angular/core/testing';
 import {BrowserModule} from '@angular/platform-browser';
 import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
@@ -148,6 +148,64 @@ export function main() {
          });
        }));
 
+    it('should initialize inputs in time for `ngOnChanges`', async(() => {
+         @Component({
+           selector: 'ng2',
+           template: `
+             ngOnChangesCount: {{ ngOnChangesCount }} |
+             firstChangesCount: {{ firstChangesCount }} |
+             initialValue: {{ initialValue }}`
+         })
+         class Ng2Component implements OnChanges {
+           ngOnChangesCount = 0;
+           firstChangesCount = 0;
+           initialValue: string;
+           @Input() foo: string;
+
+           ngOnChanges(changes: SimpleChanges) {
+             this.ngOnChangesCount++;
+
+             if (this.ngOnChangesCount === 1) {
+               this.initialValue = this.foo;
+             }
+
+             if (changes['foo'] && changes['foo'].isFirstChange()) {
+               this.firstChangesCount++;
+             }
+           }
+         }
+
+         @NgModule({
+           imports: [BrowserModule, UpgradeModule],
+           declarations: [Ng2Component],
+           entryComponents: [Ng2Component]
+         })
+         class Ng2Module {
+           ngDoBootstrap() {}
+         }
+
+         const ng1Module = angular.module('ng1', []).directive(
+             'ng2', downgradeComponent({component: Ng2Component}));
+
+         const element = html(`
+           <ng2 [foo]="'foo'"></ng2>
+           <ng2 foo="bar"></ng2>
+           <ng2 [foo]="'baz'" ng-if="true"></ng2>
+           <ng2 foo="qux" ng-if="true"></ng2>
+         `);
+
+         bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(upgrade => {
+           const nodes = element.querySelectorAll('ng2');
+           const expectedTextWith = (value: string) =>
+               `ngOnChangesCount: 1 | firstChangesCount: 1 | initialValue: ${value}`;
+
+           expect(multiTrim(nodes[0].textContent)).toBe(expectedTextWith('foo'));
+           expect(multiTrim(nodes[1].textContent)).toBe(expectedTextWith('bar'));
+           expect(multiTrim(nodes[2].textContent)).toBe(expectedTextWith('baz'));
+           expect(multiTrim(nodes[3].textContent)).toBe(expectedTextWith('qux'));
+         });
+       }));
+
     it('should bind to ng-model', async(() => {
          const ng1Module = angular.module('ng1', []).run(
              ($rootScope: angular.IScope) => { $rootScope['modelA'] = 'A'; });