fix(upgrade): use correct attribute name for upgraded component's bindings (#16128)

Previously, when using a different property/attribute name for an upgraded
component's binding (e.g. `bindings: {propName: '<attrName'}`), the property and
attribute names were swapped (e.g. using `attrName` as the property name and
`propName` as the attribute name). This resulted in unexpected behavior.

This commit fixes this using the correct names for properties and attributes.

This only affects `upgrade/dynamic`. `upgrade/static` works correctly already.

Fixes #8856

PR Close #16128

packages/upgrade/src/dynamic/upgrade_ng1_adapter.ts

@@ -101,49 +101,50 @@ export class UpgradeNg1ComponentAdapterBuilder {
     const context = (btcIsObject) ? this.directive !.bindToController : this.directive !.scope;
 
     if (typeof context == 'object') {
-      for (const name in context) {
-        if ((<any>context).hasOwnProperty(name)) {
-          let localName = context[name];
-          const type = localName.charAt(0);
-          const typeOptions = localName.charAt(1);
-          localName = typeOptions === '?' ? localName.substr(2) : localName.substr(1);
-          localName = localName || name;
+      Object.keys(context).forEach(propName => {
+        const definition = context[propName];
+        const bindingType = definition.charAt(0);
+        const bindingOptions = definition.charAt(1);
+        const attrName = definition.substring(bindingOptions === '?' ? 2 : 1) || propName;
 
-          const outputName = 'output_' + name;
-          const outputNameRename = outputName + ': ' + name;
-          const outputNameRenameChange = outputName + ': ' + name + 'Change';
-          const inputName = 'input_' + name;
-          const inputNameRename = inputName + ': ' + name;
-          switch (type) {
-            case '=':
-              this.propertyOutputs.push(outputName);
-              this.checkProperties.push(localName);
-              this.outputs.push(outputName);
-              this.outputsRename.push(outputNameRenameChange);
-              this.propertyMap[outputName] = localName;
-              this.inputs.push(inputName);
-              this.inputsRename.push(inputNameRename);
-              this.propertyMap[inputName] = localName;
-              break;
-            case '@':
-            // handle the '<' binding of angular 1.5 components
-            case '<':
-              this.inputs.push(inputName);
-              this.inputsRename.push(inputNameRename);
-              this.propertyMap[inputName] = localName;
-              break;
-            case '&':
-              this.outputs.push(outputName);
-              this.outputsRename.push(outputNameRename);
-              this.propertyMap[outputName] = localName;
-              break;
-            default:
-              let json = JSON.stringify(context);
-              throw new Error(
-                  `Unexpected mapping '${type}' in '${json}' in '${this.name}' directive.`);
-          }
+        // QUESTION: What about `=*`? Ignore? Throw? Support?
+
+        const inputName = `input_${attrName}`;
+        const inputNameRename = `${inputName}: ${attrName}`;
+        const outputName = `output_${attrName}`;
+        const outputNameRename = `${outputName}: ${attrName}`;
+        const outputNameRenameChange = `${outputNameRename}Change`;
+
+        switch (bindingType) {
+          case '@':
+          case '<':
+            this.inputs.push(inputName);
+            this.inputsRename.push(inputNameRename);
+            this.propertyMap[inputName] = propName;
+            break;
+          case '=':
+            this.inputs.push(inputName);
+            this.inputsRename.push(inputNameRename);
+            this.propertyMap[inputName] = propName;
+
+            this.outputs.push(outputName);
+            this.outputsRename.push(outputNameRenameChange);
+            this.propertyMap[outputName] = propName;
+
+            this.checkProperties.push(propName);
+            this.propertyOutputs.push(outputName);
+            break;
+          case '&':
+            this.outputs.push(outputName);
+            this.outputsRename.push(outputNameRename);
+            this.propertyMap[outputName] = propName;
+            break;
+          default:
+            let json = JSON.stringify(context);
+            throw new Error(
+                `Unexpected mapping '${bindingType}' in '${json}' in '${this.name}' directive.`);
         }
-      }
+      });
     }
   }
 
@@ -239,13 +240,11 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
       (this as any /** TODO #9100 */)[inputs[i]] = null;
     }
     for (let j = 0; j < outputs.length; j++) {
-      const emitter = (this as any /** TODO #9100 */)[outputs[j]] = new EventEmitter();
+      const emitter = (this as any)[outputs[j]] = new EventEmitter<any>();
       this.setComponentProperty(
-          outputs[j], ((emitter: any /** TODO #9100 */) => (value: any /** TODO #9100 */) =>
-                           emitter.emit(value))(emitter));
+          outputs[j], (emitter => (value: any) => emitter.emit(value))(emitter));
     }
     for (let k = 0; k < propOuts.length; k++) {
-      (this as any /** TODO #9100 */)[propOuts[k]] = new EventEmitter();
       this.checkLastValues.push(INITIAL_VALUE);
     }
   }
@@ -306,18 +305,16 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
     const destinationObj = this.destinationObj;
     const lastValues = this.checkLastValues;
     const checkProperties = this.checkProperties;
-    for (let i = 0; i < checkProperties.length; i++) {
-      const value = destinationObj ![checkProperties[i]];
+    const propOuts = this.propOuts;
+    checkProperties.forEach((propName, i) => {
+      const value = destinationObj ![propName];
       const last = lastValues[i];
-      if (value !== last) {
-        if (typeof value == 'number' && isNaN(value) && typeof last == 'number' && isNaN(last)) {
-          // ignore because NaN != NaN
-        } else {
-          const eventEmitter: EventEmitter<any> = (this as any /** TODO #9100 */)[this.propOuts[i]];
-          eventEmitter.emit(lastValues[i] = value);
-        }
+      if (value !== last &&
+          (value === value || last === last)) {  // ignore NaN values (NaN !== NaN)
+        const eventEmitter: EventEmitter<any> = (this as any)[propOuts[i]];
+        eventEmitter.emit(lastValues[i] = value);
       }
-    }
+    });
 
     if (this.controllerInstance && isFunction(this.controllerInstance.$doCheck)) {
       this.controllerInstance.$doCheck();

packages/upgrade/test/dynamic/test_helpers.ts

@@ -6,4 +6,13 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {UpgradeAdapterRef} from '@angular/upgrade';
+import * as angular from '@angular/upgrade/src/common/angular1';
+import {$ROOT_SCOPE} from '@angular/upgrade/src/common/constants';
+
 export * from '../common/test_helpers';
+
+export function $digest(adapter: UpgradeAdapterRef) {
+  const $rootScope = adapter.ng1Injector.get($ROOT_SCOPE) as angular.IRootScopeService;
+  $rootScope.$digest();
+}

packages/upgrade/test/dynamic/upgrade_spec.ts

@@ -12,7 +12,7 @@ import {BrowserModule} from '@angular/platform-browser';
 import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
 import * as angular from '@angular/upgrade/src/common/angular1';
 import {UpgradeAdapter, UpgradeAdapterRef} from '@angular/upgrade/src/dynamic/upgrade_adapter';
-import {html, multiTrim} from './test_helpers';
+import {$digest, html, multiTrim} from './test_helpers';
 
 export function main() {
   describe('adapter: ng1 to ng2', () => {
@@ -607,22 +607,268 @@ export function main() {
     });
 
     describe('upgrade ng1 component', () => {
+      it('should support `@` bindings', fakeAsync(() => {
+           const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
+           let ng2ComponentInstance: Ng2Component;
+
+           // Define `ng1Component`
+           const ng1Component: angular.IComponent = {
+             template: 'Inside: {{ $ctrl.inputA }}, {{ $ctrl.inputB }}',
+             bindings: {inputA: '@inputAttrA', inputB: '@'}
+           };
+
+           // Define `Ng2Component`
+           @Component({
+             selector: 'ng2',
+             template: `
+               <ng1 inputAttrA="{{ dataA }}" inputB="{{ dataB }}"></ng1>
+               | Outside: {{ dataA }}, {{ dataB }}
+             `
+           })
+           class Ng2Component {
+             dataA = 'foo';
+             dataB = 'bar';
+
+             constructor() { ng2ComponentInstance = this; }
+           }
+
+           // Define `ng1Module`
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2', adapter.downgradeNg2Component(Ng2Component));
+
+           // Define `Ng2Module`
+           @NgModule({
+             declarations: [adapter.upgradeNg1Component('ng1'), Ng2Component],
+             imports: [BrowserModule]
+           })
+           class Ng2Module {
+           }
+
+           // Bootstrap
+           const element = html(`<ng2></ng2>`);
+
+           adapter.bootstrap(element, ['ng1Module']).ready(ref => {
+             const ng1 = element.querySelector('ng1') !;
+             const ng1Controller = angular.element(ng1).controller !('ng1');
+
+             expect(multiTrim(element.textContent)).toBe('Inside: foo, bar | Outside: foo, bar');
+
+             ng1Controller.inputA = 'baz';
+             ng1Controller.inputB = 'qux';
+             $digest(ref);
+
+             expect(multiTrim(element.textContent)).toBe('Inside: baz, qux | Outside: foo, bar');
+
+             ng2ComponentInstance.dataA = 'foo2';
+             ng2ComponentInstance.dataB = 'bar2';
+             $digest(ref);
+
+             expect(multiTrim(element.textContent))
+                 .toBe('Inside: foo2, bar2 | Outside: foo2, bar2');
+
+             ref.dispose();
+           });
+         }));
+
+      it('should support `<` bindings', fakeAsync(() => {
+           const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
+           let ng2ComponentInstance: Ng2Component;
+
+           // Define `ng1Component`
+           const ng1Component: angular.IComponent = {
+             template: 'Inside: {{ $ctrl.inputA.value }}, {{ $ctrl.inputB.value }}',
+             bindings: {inputA: '<inputAttrA', inputB: '<'}
+           };
+
+           // Define `Ng2Component`
+           @Component({
+             selector: 'ng2',
+             template: `
+               <ng1 [inputAttrA]="dataA" [inputB]="dataB"></ng1>
+               | Outside: {{ dataA.value }}, {{ dataB.value }}
+             `
+           })
+           class Ng2Component {
+             dataA = {value: 'foo'};
+             dataB = {value: 'bar'};
+
+             constructor() { ng2ComponentInstance = this; }
+           }
+
+           // Define `ng1Module`
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2', adapter.downgradeNg2Component(Ng2Component));
+
+           // Define `Ng2Module`
+           @NgModule({
+             declarations: [adapter.upgradeNg1Component('ng1'), Ng2Component],
+             imports: [BrowserModule]
+           })
+           class Ng2Module {
+           }
+
+           // Bootstrap
+           const element = html(`<ng2></ng2>`);
+
+           adapter.bootstrap(element, ['ng1Module']).ready(ref => {
+             const ng1 = element.querySelector('ng1') !;
+             const ng1Controller = angular.element(ng1).controller !('ng1');
+
+             expect(multiTrim(element.textContent)).toBe('Inside: foo, bar | Outside: foo, bar');
+
+             ng1Controller.inputA = {value: 'baz'};
+             ng1Controller.inputB = {value: 'qux'};
+             $digest(ref);
+
+             expect(multiTrim(element.textContent)).toBe('Inside: baz, qux | Outside: foo, bar');
+
+             ng2ComponentInstance.dataA = {value: 'foo2'};
+             ng2ComponentInstance.dataB = {value: 'bar2'};
+             $digest(ref);
+
+             expect(multiTrim(element.textContent))
+                 .toBe('Inside: foo2, bar2 | Outside: foo2, bar2');
+
+             ref.dispose();
+           });
+         }));
+
+      it('should support `=` bindings', fakeAsync(() => {
+           const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
+           let ng2ComponentInstance: Ng2Component;
+
+           // Define `ng1Component`
+           const ng1Component: angular.IComponent = {
+             template: 'Inside: {{ $ctrl.inputA.value }}, {{ $ctrl.inputB.value }}',
+             bindings: {inputA: '=inputAttrA', inputB: '='}
+           };
+
+           // Define `Ng2Component`
+           @Component({
+             selector: 'ng2',
+             template: `
+               <ng1 [(inputAttrA)]="dataA" [(inputB)]="dataB"></ng1>
+               | Outside: {{ dataA.value }}, {{ dataB.value }}
+             `
+           })
+           class Ng2Component {
+             dataA = {value: 'foo'};
+             dataB = {value: 'bar'};
+
+             constructor() { ng2ComponentInstance = this; }
+           }
+
+           // Define `ng1Module`
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2', adapter.downgradeNg2Component(Ng2Component));
+
+           // Define `Ng2Module`
+           @NgModule({
+             declarations: [adapter.upgradeNg1Component('ng1'), Ng2Component],
+             imports: [BrowserModule]
+           })
+           class Ng2Module {
+           }
+
+           // Bootstrap
+           const element = html(`<ng2></ng2>`);
+
+           adapter.bootstrap(element, ['ng1Module']).ready(ref => {
+             const ng1 = element.querySelector('ng1') !;
+             const ng1Controller = angular.element(ng1).controller !('ng1');
+
+             expect(multiTrim(element.textContent)).toBe('Inside: foo, bar | Outside: foo, bar');
+
+             ng1Controller.inputA = {value: 'baz'};
+             ng1Controller.inputB = {value: 'qux'};
+             $digest(ref);
+
+             expect(multiTrim(element.textContent)).toBe('Inside: baz, qux | Outside: baz, qux');
+
+             ng2ComponentInstance.dataA = {value: 'foo2'};
+             ng2ComponentInstance.dataB = {value: 'bar2'};
+             $digest(ref);
+
+             expect(multiTrim(element.textContent))
+                 .toBe('Inside: foo2, bar2 | Outside: foo2, bar2');
+
+             ref.dispose();
+           });
+         }));
+
+      it('should support `&` bindings', fakeAsync(() => {
+           const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
+
+           // Define `ng1Component`
+           const ng1Component: angular.IComponent = {
+             template: 'Inside: -',
+             bindings: {outputA: '&outputAttrA', outputB: '&'}
+           };
+
+           // Define `Ng2Component`
+           @Component({
+             selector: 'ng2',
+             template: `
+               <ng1 (outputAttrA)="dataA = $event" (outputB)="dataB = $event"></ng1>
+               | Outside: {{ dataA }}, {{ dataB }}
+             `
+           })
+           class Ng2Component {
+             dataA = 'foo';
+             dataB = 'bar';
+           }
+
+           // Define `ng1Module`
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2', adapter.downgradeNg2Component(Ng2Component));
+
+           // Define `Ng2Module`
+           @NgModule({
+             declarations: [adapter.upgradeNg1Component('ng1'), Ng2Component],
+             imports: [BrowserModule]
+           })
+           class Ng2Module {
+           }
+
+           // Bootstrap
+           const element = html(`<ng2></ng2>`);
+
+           adapter.bootstrap(element, ['ng1Module']).ready(ref => {
+             const ng1 = element.querySelector('ng1') !;
+             const ng1Controller = angular.element(ng1).controller !('ng1');
+
+             expect(multiTrim(element.textContent)).toBe('Inside: - | Outside: foo, bar');
+
+             ng1Controller.outputA('baz');
+             ng1Controller.outputB('qux');
+             $digest(ref);
+
+             expect(multiTrim(element.textContent)).toBe('Inside: - | Outside: baz, qux');
+
+             ref.dispose();
+           });
+         }));
+
       it('should bind properties, events', async(() => {
            const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
            const ng1Module = angular.module('ng1', []);
 
            const ng1 = () => {
              return {
-               template: 'Hello {{fullName}}; A: {{dataA}}; B: {{dataB}}; C: {{modelC}}; | ',
+               template: 'Hello {{fullName}}; A: {{modelA}}; B: {{modelB}}; C: {{modelC}}; | ',
                scope: {fullName: '@', modelA: '=dataA', modelB: '=dataB', modelC: '=', event: '&'},
                link: function(scope: any) {
-                 scope.$watch('dataB', (v: string) => {
+                 scope.$watch('modelB', (v: string) => {
                    if (v == 'Savkin') {
-                     scope.dataB = 'SAVKIN';
+                     scope.modelB = 'SAVKIN';
                      scope.event('WORKS');
 
                      // Should not update because [model-a] is uni directional
-                     scope.dataA = 'VICTOR';
+                     scope.modelA = 'VICTOR';
                    }
                  });
                }
@@ -633,9 +879,9 @@ export function main() {
                Component({
                  selector: 'ng2',
                  template:
-                     '<ng1 fullName="{{last}}, {{first}}, {{city}}" [modelA]="first" [(modelB)]="last" [modelC]="city" ' +
+                     '<ng1 fullName="{{last}}, {{first}}, {{city}}" [dataA]="first" [(dataB)]="last" [modelC]="city" ' +
                      '(event)="event=$event"></ng1>' +
-                     '<ng1 fullName="{{\'TEST\'}}" modelA="First" modelB="Last" modelC="City"></ng1>' +
+                     '<ng1 fullName="{{\'TEST\'}}" dataA="First" dataB="Last" modelC="City"></ng1>' +
                      '{{event}}-{{last}}, {{first}}, {{city}}'
                }).Class({
                  constructor: function() {
@@ -671,15 +917,15 @@ export function main() {
 
            const ng1 = () => {
              return {
-               template: 'Hello; A: {{dataA}}; B: {{modelB}}; | ',
+               template: 'Hello; A: {{modelA}}; B: {{modelB}}; | ',
                scope: {modelA: '=?dataA', modelB: '=?'}
              };
            };
            ng1Module.directive('ng1', ng1);
            const Ng2 = Component({
                          selector: 'ng2',
-                         template: '<ng1 [modelA]="first" [modelB]="last"></ng1>' +
-                             '<ng1 modelA="First" modelB="Last"></ng1>' +
+                         template: '<ng1 [dataA]="first" [modelB]="last"></ng1>' +
+                             '<ng1 dataA="First" modelB="Last"></ng1>' +
                              '<ng1></ng1>' +
                              '<ng1></ng1>'
                        }).Class({