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
This commit is contained in:
Georgios Kalpakas 2017-02-04 17:19:09 +02:00 committed by Miško Hevery
parent 701074cf89
commit 1e3dd3dd9b
5 changed files with 88 additions and 58 deletions

View File

@ -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;
}
}

View File

@ -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(`<div>
<ng2 literal="Text" interpolate="Hello {{'world'}}"
<ng2 literal="Text" interpolate="Hello {{name}}"
bind-one-way-a="dataA" [one-way-b]="dataB"
bindon-two-way-a="modelA" [(two-way-b)]="modelB"
on-event-a='eventA=$event' (event-b)="eventB=$event"></ng2>
@ -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();
});

View File

@ -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(`
<div>
<ng2 literal="Text" interpolate="Hello {{'world'}}"
<ng2 literal="Text" interpolate="Hello {{name}}"
bind-one-way-a="dataA" [one-way-b]="dataB"
bindon-two-way-a="modelA" [(two-way-b)]="modelB"
on-event-a='eventA=$event' (event-b)="eventB=$event"></ng2>
@ -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;');
});
}));

View File

@ -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))

View File

@ -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();
}