From 1e3dd3dd9bcd7940d721c651047a03a0e6ed7d5c Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Sat, 4 Feb 2017 17:19:09 +0200 Subject: [PATCH] fix(upgrade): pass correct values to `ngOnChanges` for interpolation bindings (#14301) Previously, the `previousValue` and `currentValue` arguments passed to the `SimpleChange` constructor were swapped for interpolation bindings. This commit also refactors the code, so that interpolation bindings and property bindings share the same implementation, and fixes some broken tests (that hide failures by allowing the `$exceptionHandler` to swallow thrown exceptions). PR Close #14301 --- .../src/common/downgrade_component_adapter.ts | 35 ++++++------- .../upgrade/test/dynamic/upgrade_spec.ts | 18 +++++-- .../integration/downgrade_component_spec.ts | 36 ++++++++----- .../integration/upgrade_component_spec.ts | 50 +++++++++---------- .../upgrade/test/static/test_helpers.ts | 7 ++- 5 files changed, 88 insertions(+), 58 deletions(-) diff --git a/modules/@angular/upgrade/src/common/downgrade_component_adapter.ts b/modules/@angular/upgrade/src/common/downgrade_component_adapter.ts index c6549e8923..fa536a47e2 100644 --- a/modules/@angular/upgrade/src/common/downgrade_component_adapter.ts +++ b/modules/@angular/upgrade/src/common/downgrade_component_adapter.ts @@ -78,17 +78,15 @@ export class DowngradeComponentAdapter { let expr: any /** TODO #9100 */ = null; if (attrs.hasOwnProperty(input.attr)) { - const observeFn = ((prop: any /** TODO #9100 */) => { + const observeFn = (prop => { let prevValue = INITIAL_VALUE; - return (value: any /** TODO #9100 */) => { - if (this.inputChanges !== null) { - this.inputChangeCount++; - this.inputChanges[prop] = new SimpleChange( - value, prevValue === INITIAL_VALUE ? value : prevValue, - prevValue === INITIAL_VALUE); - prevValue = value; + return (currValue: any) => { + if (prevValue === INITIAL_VALUE) { + prevValue = currValue; } - this.component[prop] = value; + + this.updateInput(prop, prevValue, currValue); + prevValue = currValue; }; })(input.prop); attrs.$observe(input.attr, observeFn); @@ -104,14 +102,8 @@ export class DowngradeComponentAdapter { } if (expr != null) { const watchFn = - ((prop: any /** TODO #9100 */) => ( - value: any /** TODO #9100 */, prevValue: any /** TODO #9100 */) => { - if (this.inputChanges != null) { - this.inputChangeCount++; - this.inputChanges[prop] = new SimpleChange(prevValue, value, prevValue === value); - } - this.component[prop] = value; - })(input.prop); + (prop => (currValue: any, prevValue: any) => + this.updateInput(prop, prevValue, currValue))(input.prop); this.componentScope.$watch(expr, watchFn); } } @@ -185,4 +177,13 @@ export class DowngradeComponentAdapter { } getInjector(): Injector { return this.componentRef && this.componentRef.injector; } + + private updateInput(prop: string, prevValue: any, currValue: any) { + if (this.inputChanges) { + this.inputChangeCount++; + this.inputChanges[prop] = new SimpleChange(prevValue, currValue, prevValue === currValue); + } + + this.component[prop] = currValue; + } } diff --git a/modules/@angular/upgrade/test/dynamic/upgrade_spec.ts b/modules/@angular/upgrade/test/dynamic/upgrade_spec.ts index 1db48add84..db05c40801 100644 --- a/modules/@angular/upgrade/test/dynamic/upgrade_spec.ts +++ b/modules/@angular/upgrade/test/dynamic/upgrade_spec.ts @@ -290,9 +290,11 @@ export function main() { it('should bind properties, events', async(() => { const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module)); - const ng1Module = angular.module('ng1', []); + const ng1Module = + angular.module('ng1', []).value('$exceptionHandler', (err: any) => { throw err; }); ng1Module.run(($rootScope: any) => { + $rootScope.name = 'world'; $rootScope.dataA = 'A'; $rootScope.dataB = 'B'; $rootScope.modelA = 'initModelA'; @@ -363,9 +365,10 @@ export function main() { break; case 1: assertChange('twoWayA', 'newA'); + assertChange('twoWayB', 'newB'); break; case 2: - assertChange('twoWayB', 'newB'); + assertChange('interpolate', 'Hello everyone'); break; default: throw new Error('Called too many times! ' + JSON.stringify(changes)); @@ -380,7 +383,7 @@ export function main() { }).Class({constructor: function() {}}); const element = html(`
- @@ -393,6 +396,15 @@ export function main() { 'literal: Text; interpolate: Hello world; ' + 'oneWayA: A; oneWayB: B; twoWayA: newA; twoWayB: newB; (2) | ' + 'modelA: newA; modelB: newB; eventA: aFired; eventB: bFired;'); + + ref.ng1RootScope.$apply('name = "everyone"'); + expect(multiTrim(document.body.textContent)) + .toEqual( + 'ignore: -; ' + + 'literal: Text; interpolate: Hello everyone; ' + + 'oneWayA: A; oneWayB: B; twoWayA: newA; twoWayB: newB; (3) | ' + + 'modelA: newA; modelB: newB; eventA: aFired; eventB: bFired;'); + ref.dispose(); }); diff --git a/modules/@angular/upgrade/test/static/integration/downgrade_component_spec.ts b/modules/@angular/upgrade/test/static/integration/downgrade_component_spec.ts index 25783719e2..59339416dc 100644 --- a/modules/@angular/upgrade/test/static/integration/downgrade_component_spec.ts +++ b/modules/@angular/upgrade/test/static/integration/downgrade_component_spec.ts @@ -13,7 +13,7 @@ import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; import * as angular from '@angular/upgrade/src/common/angular1'; import {UpgradeModule, downgradeComponent} from '@angular/upgrade/static'; -import {bootstrap, html, multiTrim} from '../test_helpers'; +import {$apply, bootstrap, html, multiTrim} from '../test_helpers'; export function main() { describe('downgrade ng2 component', () => { @@ -22,15 +22,18 @@ export function main() { afterEach(() => destroyPlatform()); it('should bind properties, events', async(() => { - - const ng1Module = angular.module('ng1', []).run(($rootScope: angular.IScope) => { - $rootScope['dataA'] = 'A'; - $rootScope['dataB'] = 'B'; - $rootScope['modelA'] = 'initModelA'; - $rootScope['modelB'] = 'initModelB'; - $rootScope['eventA'] = '?'; - $rootScope['eventB'] = '?'; - }); + const ng1Module = + angular.module('ng1', []).value('$exceptionHandler', (err: any) => { + throw err; + }).run(($rootScope: angular.IScope) => { + $rootScope['name'] = 'world'; + $rootScope['dataA'] = 'A'; + $rootScope['dataB'] = 'B'; + $rootScope['modelA'] = 'initModelA'; + $rootScope['modelB'] = 'initModelB'; + $rootScope['eventA'] = '?'; + $rootScope['eventB'] = '?'; + }); @Component({ selector: 'ng2', @@ -94,9 +97,10 @@ export function main() { break; case 1: assertChange('twoWayA', 'newA'); + assertChange('twoWayB', 'newB'); break; case 2: - assertChange('twoWayB', 'newB'); + assertChange('interpolate', 'Hello everyone'); break; default: throw new Error('Called too many times! ' + JSON.stringify(changes)); @@ -125,7 +129,7 @@ export function main() { const element = html(`
- @@ -139,6 +143,14 @@ export function main() { 'literal: Text; interpolate: Hello world; ' + 'oneWayA: A; oneWayB: B; twoWayA: newA; twoWayB: newB; (2) | ' + 'modelA: newA; modelB: newB; eventA: aFired; eventB: bFired;'); + + $apply(upgrade, 'name = "everyone"'); + expect(multiTrim(document.body.textContent)) + .toEqual( + 'ignore: -; ' + + 'literal: Text; interpolate: Hello everyone; ' + + 'oneWayA: A; oneWayB: B; twoWayA: newA; twoWayB: newB; (3) | ' + + 'modelA: newA; modelB: newB; eventA: aFired; eventB: bFired;'); }); })); diff --git a/modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts b/modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts index 812f55f8a8..855e9ede00 100644 --- a/modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts +++ b/modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts @@ -13,7 +13,7 @@ import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; import * as angular from '@angular/upgrade/src/common/angular1'; import {UpgradeComponent, UpgradeModule, downgradeComponent} from '@angular/upgrade/static'; -import {bootstrap, digest, html, multiTrim} from '../test_helpers'; +import {$digest, bootstrap, html, multiTrim} from '../test_helpers'; export function main() { describe('upgrade ng1 component', () => { @@ -530,7 +530,7 @@ export function main() { ng2ComponentInstance.dataA = 'foo2'; ng2ComponentInstance.dataB = 'bar2'; - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(element.textContent)) @@ -605,7 +605,7 @@ export function main() { ng2ComponentInstance.dataA = {value: 'foo2'}; ng2ComponentInstance.dataB = {value: 'bar2'}; - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(element.textContent)) @@ -682,7 +682,7 @@ export function main() { ng2ComponentInstance.dataA = {value: 'foo2'}; ng2ComponentInstance.dataB = {value: 'bar2'}; - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(element.textContent)) @@ -929,7 +929,7 @@ export function main() { ng1Controller1.outputA({value: 'foo again'}); ng1Controller2.outputB('bar again'); - digest(adapter); + $digest(adapter); tick(); expect(ng1Controller0.inputA).toEqual({value: 'foo again'}); @@ -1010,7 +1010,7 @@ export function main() { .toBe('ng1 - Data: [4,5] - Length: 2 | ng2 - Data: 4,5 - Length: 2'); ng1Controller.$scope.outputA(6); - digest(adapter); + $digest(adapter); tick(); expect(ng1Controller.$scope.inputA).toEqual([4, 5, 6]); @@ -1858,7 +1858,7 @@ export function main() { // Change: Re-assign `data` ng2ComponentInstance.data = {foo: 'baz'}; - digest(adapter); + $digest(adapter); tick(); expect(scopeOnChanges.calls.count()).toBe(2); @@ -1879,7 +1879,7 @@ export function main() { // No change: Update internal property ng2ComponentInstance.data.foo = 'qux'; - digest(adapter); + $digest(adapter); tick(); expect(scopeOnChanges.calls.count()).toBe(2); @@ -1888,7 +1888,7 @@ export function main() { // Change: Re-assign `data` (even if it looks the same) ng2ComponentInstance.data = {foo: 'qux'}; - digest(adapter); + $digest(adapter); tick(); expect(scopeOnChanges.calls.count()).toBe(3); @@ -2008,7 +2008,7 @@ export function main() { // Change: Re-assign `data` ng2ComponentInstance.data = {foo: 'baz'}; - digest(adapter); + $digest(adapter); tick(); expect(scopeOnChangesA.calls.count()).toBe(2); @@ -2029,7 +2029,7 @@ export function main() { // No change: Update internal property ng2ComponentInstance.data.foo = 'qux'; - digest(adapter); + $digest(adapter); tick(); expect(scopeOnChangesA.calls.count()).toBe(2); @@ -2039,7 +2039,7 @@ export function main() { // Change: Re-assign `data` (even if it looks the same) ng2ComponentInstance.data = {foo: 'qux'}; - digest(adapter); + $digest(adapter); tick(); expect(scopeOnChangesA.calls.count()).toBe(3); @@ -2399,12 +2399,12 @@ export function main() { // Run a `$digest` // (Since it's the first one since the `$doCheck` watcher was added, // the `watchFn` will be run twice.) - digest(adapter); + $digest(adapter); expect(controllerDoCheckA.calls.count()).toBe(3); expect(controllerDoCheckB.calls.count()).toBe(3); // Run another `$digest` - digest(adapter); + $digest(adapter); expect(controllerDoCheckA.calls.count()).toBe(4); expect(controllerDoCheckB.calls.count()).toBe(4); }); @@ -2474,11 +2474,11 @@ export function main() { expect(scopeDoCheck).not.toHaveBeenCalled(); // Run a `$digest` - digest(adapter); + $digest(adapter); expect(scopeDoCheck).not.toHaveBeenCalled(); // Run another `$digest` - digest(adapter); + $digest(adapter); expect(scopeDoCheck).not.toHaveBeenCalled(); }); })); @@ -2788,7 +2788,7 @@ export function main() { expect(scopeDestroyListener).not.toHaveBeenCalled(); ng2ComponentAInstance.destroyIt = true; - digest(adapter); + $digest(adapter); expect(scopeDestroyListener).toHaveBeenCalled(); }); @@ -2952,7 +2952,7 @@ export function main() { // (Should not propagate upwards.) ng2ComponentBInstance.ng2BInputA = 'foo2'; ng2ComponentBInstance.ng2BInputC = 'baz2'; - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(document.body.textContent)) @@ -2962,7 +2962,7 @@ export function main() { // (Should propagate all the way up to `ng1ADataC` and back all the way down to // `ng2BInputC`.) ng2ComponentBInstance.ng2BOutputC.emit('baz3'); - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(document.body.textContent)) @@ -2972,7 +2972,7 @@ export function main() { // (Should not propagate upwards, only downwards.) ng1ControllerXInstance.ng1XInputA = 'foo4'; ng1ControllerXInstance.ng1XInputB = {value: 'bar4'}; - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(document.body.textContent)) @@ -2981,7 +2981,7 @@ export function main() { // Update `ng1XInputC`. // (Should propagate upwards and downwards.) ng1ControllerXInstance.ng1XInputC = {value: 'baz5'}; - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(document.body.textContent)) @@ -2990,7 +2990,7 @@ export function main() { // Update a property on `ng1XInputC`. // (Should propagate upwards and downwards.) ng1ControllerXInstance.ng1XInputC.value = 'baz6'; - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(document.body.textContent)) @@ -2999,7 +2999,7 @@ export function main() { // Emit from `ng1XOutputA`. // (Should propagate upwards to `ng1ADataA` and back all the way down to `ng2BInputA`.) (ng1ControllerXInstance as any).ng1XOutputA({value: 'foo7'}); - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(document.body.textContent)) @@ -3009,7 +3009,7 @@ export function main() { // (Should propagate upwards to `ng1ADataB`, but not downwards, // since `ng1XInputB` has been re-assigned (i.e. `ng2ADataB !== ng1XInputB`).) (ng1ControllerXInstance as any).ng1XOutputB('bar8'); - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(document.body.textContent)) @@ -3020,7 +3020,7 @@ export function main() { ng2ComponentAInstance.ng2ADataA = {value: 'foo9'}; ng2ComponentAInstance.ng2ADataB = {value: 'bar9'}; ng2ComponentAInstance.ng2ADataC = {value: 'baz9'}; - digest(adapter); + $digest(adapter); tick(); expect(multiTrim(document.body.textContent)) diff --git a/modules/@angular/upgrade/test/static/test_helpers.ts b/modules/@angular/upgrade/test/static/test_helpers.ts index 14edabe1d9..903d3732fb 100644 --- a/modules/@angular/upgrade/test/static/test_helpers.ts +++ b/modules/@angular/upgrade/test/static/test_helpers.ts @@ -24,7 +24,12 @@ export function bootstrap( }); } -export function digest(adapter: UpgradeModule) { +export function $apply(adapter: UpgradeModule, exp: angular.Ng1Expression) { + const $rootScope = adapter.$injector.get($ROOT_SCOPE) as angular.IRootScopeService; + $rootScope.$apply(exp); +} + +export function $digest(adapter: UpgradeModule) { const $rootScope = adapter.$injector.get($ROOT_SCOPE) as angular.IRootScopeService; $rootScope.$digest(); }