Skip to content

Commit 5391f96

Browse files
FDIMmhevery
FDIM
authored and
mhevery
committed
fix(upgrade): two-way binding and listening for event (#22772)
Changes would not propagate to a value in downgraded component in case you had two-way binding and listening to a value-change, e.g. [(value)]="value" (value-change)="fetch()" Closes #22734 PR Close #22772
1 parent aca4735 commit 5391f96

File tree

3 files changed

+131
-32
lines changed

3 files changed

+131
-32
lines changed

‎packages/upgrade/src/common/downgrade_component_adapter.ts

+28-30
Original file line numberDiff line numberDiff line change
@@ -168,45 +168,43 @@ export class DowngradeComponentAdapter {
168168
const outputs = this.componentFactory.outputs || [];
169169
for (let j = 0; j < outputs.length; j++) {
170170
const output = new PropertyBinding(outputs[j].propName, outputs[j].templateName);
171-
let expr: string|null = null;
172-
let assignExpr = false;
173-
174171
const bindonAttr = output.bindonAttr.substring(0, output.bindonAttr.length - 6);
175172
const bracketParenAttr =
176173
`[(${output.bracketParenAttr.substring(2, output.bracketParenAttr.length - 8)})]`;
177-
174+
// order below is important - first update bindings then evaluate expressions
175+
if (attrs.hasOwnProperty(bindonAttr)) {
176+
this.subscribeToOutput(output, attrs[bindonAttr], true);
177+
}
178+
if (attrs.hasOwnProperty(bracketParenAttr)) {
179+
this.subscribeToOutput(output, attrs[bracketParenAttr], true);
180+
}
178181
if (attrs.hasOwnProperty(output.onAttr)) {
179-
expr = attrs[output.onAttr];
180-
} else if (attrs.hasOwnProperty(output.parenAttr)) {
181-
expr = attrs[output.parenAttr];
182-
} else if (attrs.hasOwnProperty(bindonAttr)) {
183-
expr = attrs[bindonAttr];
184-
assignExpr = true;
185-
} else if (attrs.hasOwnProperty(bracketParenAttr)) {
186-
expr = attrs[bracketParenAttr];
187-
assignExpr = true;
182+
this.subscribeToOutput(output, attrs[output.onAttr]);
188183
}
189-
190-
if (expr != null && assignExpr != null) {
191-
const getter = this.$parse(expr);
192-
const setter = getter.assign;
193-
if (assignExpr && !setter) {
194-
throw new Error(`Expression '${expr}' is not assignable!`);
195-
}
196-
const emitter = this.component[output.prop] as EventEmitter<any>;
197-
if (emitter) {
198-
emitter.subscribe({
199-
next: assignExpr ? (v: any) => setter !(this.scope, v) :
200-
(v: any) => getter(this.scope, {'$event': v})
201-
});
202-
} else {
203-
throw new Error(
204-
`Missing emitter '${output.prop}' on component '${getComponentName(this.componentFactory.componentType)}'!`);
205-
}
184+
if (attrs.hasOwnProperty(output.parenAttr)) {
185+
this.subscribeToOutput(output, attrs[output.parenAttr]);
206186
}
207187
}
208188
}
209189

190+
private subscribeToOutput(output: PropertyBinding, expr: string, isAssignment: boolean = false) {
191+
const getter = this.$parse(expr);
192+
const setter = getter.assign;
193+
if (isAssignment && !setter) {
194+
throw new Error(`Expression '${expr}' is not assignable!`);
195+
}
196+
const emitter = this.component[output.prop] as EventEmitter<any>;
197+
if (emitter) {
198+
emitter.subscribe({
199+
next: isAssignment ? (v: any) => setter !(this.scope, v) :
200+
(v: any) => getter(this.scope, {'$event': v})
201+
});
202+
} else {
203+
throw new Error(
204+
`Missing emitter '${output.prop}' on component '${getComponentName(this.componentFactory.componentType)}'!`);
205+
}
206+
}
207+
210208
registerCleanup() {
211209
const destroyComponentRef = this.wrapCallback(() => this.componentRef.destroy());
212210
let destroyed = false;

‎packages/upgrade/test/dynamic/upgrade_spec.ts

+50-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ChangeDetectorRef, Component, EventEmitter, Input, NO_ERRORS_SCHEMA, NgModule, NgModuleFactory, NgZone, OnChanges, OnDestroy, SimpleChange, SimpleChanges, Testability, destroyPlatform, forwardRef} from '@angular/core';
9+
import {ChangeDetectorRef, Component, EventEmitter, Input, NO_ERRORS_SCHEMA, NgModule, NgModuleFactory, NgZone, OnChanges, OnDestroy, Output, SimpleChange, SimpleChanges, Testability, destroyPlatform, forwardRef} from '@angular/core';
1010
import {async, fakeAsync, flushMicrotasks, tick} from '@angular/core/testing';
1111
import {BrowserModule} from '@angular/platform-browser';
1212
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
@@ -434,6 +434,55 @@ withEachNg1Version(() => {
434434

435435
}));
436436

437+
it('should support two-way binding and event listener', async(() => {
438+
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
439+
const listenerSpy = jasmine.createSpy('$rootScope.listener');
440+
const ng1Module = angular.module('ng1', []).run(($rootScope: angular.IScope) => {
441+
$rootScope['value'] = 'world';
442+
$rootScope['listener'] = listenerSpy;
443+
});
444+
445+
@Component({selector: 'ng2', template: `model: {{model}};`})
446+
class Ng2Component implements OnChanges {
447+
ngOnChangesCount = 0;
448+
@Input() model = '?';
449+
@Output() modelChange = new EventEmitter();
450+
451+
ngOnChanges(changes: SimpleChanges) {
452+
switch (this.ngOnChangesCount++) {
453+
case 0:
454+
expect(changes.model.currentValue).toBe('world');
455+
this.modelChange.emit('newC');
456+
break;
457+
case 1:
458+
expect(changes.model.currentValue).toBe('newC');
459+
break;
460+
default:
461+
throw new Error('Called too many times! ' + JSON.stringify(changes));
462+
}
463+
}
464+
}
465+
466+
ng1Module.directive('ng2', adapter.downgradeNg2Component(Ng2Component));
467+
468+
@NgModule({declarations: [Ng2Component], imports: [BrowserModule]})
469+
class Ng2Module {
470+
ngDoBootstrap() {}
471+
}
472+
473+
const element = html(`
474+
<div>
475+
<ng2 [(model)]="value" (model-change)="listener($event)"></ng2>
476+
| value: {{value}}
477+
</div>
478+
`);
479+
adapter.bootstrap(element, ['ng1']).ready((ref) => {
480+
expect(multiTrim(element.textContent)).toEqual('model: newC; | value: newC');
481+
expect(listenerSpy).toHaveBeenCalledWith('newC');
482+
ref.dispose();
483+
});
484+
}));
485+
437486
it('should initialize inputs in time for `ngOnChanges`', async(() => {
438487
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
439488

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

+53-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ChangeDetectorRef, Compiler, Component, ComponentFactoryResolver, Directive, ElementRef, EventEmitter, Injector, Input, NgModule, NgModuleRef, OnChanges, OnDestroy, SimpleChanges, destroyPlatform} from '@angular/core';
9+
import {ChangeDetectorRef, Compiler, Component, ComponentFactoryResolver, Directive, ElementRef, EventEmitter, Injector, Input, NgModule, NgModuleRef, OnChanges, OnDestroy, Output, SimpleChanges, destroyPlatform} from '@angular/core';
1010
import {async, fakeAsync, tick} from '@angular/core/testing';
1111
import {BrowserModule} from '@angular/platform-browser';
1212
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
@@ -148,6 +148,58 @@ withEachNg1Version(() => {
148148
});
149149
}));
150150

151+
it('should support two-way binding and event listener', async(() => {
152+
const listenerSpy = jasmine.createSpy('$rootScope.listener');
153+
const ng1Module = angular.module('ng1', []).run(($rootScope: angular.IScope) => {
154+
$rootScope['value'] = 'world';
155+
$rootScope['listener'] = listenerSpy;
156+
});
157+
158+
@Component({selector: 'ng2', template: `model: {{model}};`})
159+
class Ng2Component implements OnChanges {
160+
ngOnChangesCount = 0;
161+
@Input() model = '?';
162+
@Output() modelChange = new EventEmitter();
163+
164+
ngOnChanges(changes: SimpleChanges) {
165+
switch (this.ngOnChangesCount++) {
166+
case 0:
167+
expect(changes.model.currentValue).toBe('world');
168+
this.modelChange.emit('newC');
169+
break;
170+
case 1:
171+
expect(changes.model.currentValue).toBe('newC');
172+
break;
173+
default:
174+
throw new Error('Called too many times! ' + JSON.stringify(changes));
175+
}
176+
}
177+
}
178+
179+
ng1Module.directive('ng2', downgradeComponent({component: Ng2Component}));
180+
181+
@NgModule({
182+
declarations: [Ng2Component],
183+
entryComponents: [Ng2Component],
184+
imports: [BrowserModule, UpgradeModule]
185+
})
186+
class Ng2Module {
187+
ngDoBootstrap() {}
188+
}
189+
190+
const element = html(`
191+
<div>
192+
<ng2 [(model)]="value" (model-change)="listener($event)"></ng2>
193+
| value: {{value}}
194+
</div>
195+
`);
196+
197+
bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then((upgrade) => {
198+
expect(multiTrim(element.textContent)).toEqual('model: newC; | value: newC');
199+
expect(listenerSpy).toHaveBeenCalledWith('newC');
200+
});
201+
}));
202+
151203
it('should run change-detection on every digest (by default)', async(() => {
152204
let ng2Component: Ng2Component;
153205

0 commit comments

Comments
 (0)