From d1fb066d0747742bf5e9854a96ab9d6999dc1534 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Tue, 18 Apr 2017 16:49:35 +0300 Subject: [PATCH] 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: '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(); 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 = (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 = (this as any)[propOuts[i]]; + eventEmitter.emit(lastValues[i] = value); } - } + }); if (this.controllerInstance && isFunction(this.controllerInstance.$doCheck)) { this.controllerInstance.$doCheck(); diff --git a/packages/upgrade/test/dynamic/test_helpers.ts b/packages/upgrade/test/dynamic/test_helpers.ts index d1676d7569..284ef8bb6f 100644 --- a/packages/upgrade/test/dynamic/test_helpers.ts +++ b/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(); +} diff --git a/packages/upgrade/test/dynamic/upgrade_spec.ts b/packages/upgrade/test/dynamic/upgrade_spec.ts index 4a501ff071..29a2ed0d8c 100644 --- a/packages/upgrade/test/dynamic/upgrade_spec.ts +++ b/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: ` + + | 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(``); + + 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: ' + | 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(``); + + 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: ` + + | 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(``); + + 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: ` + + | 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(``); + + 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: - '' + - '' + + '' + '{{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: '' + - '' + + template: '' + + '' + '' + '' }).Class({