From 5391f964066cb6f811fac2d80c325656aac8a8ac Mon Sep 17 00:00:00 2001 From: Domas Trijonis Date: Fri, 16 Mar 2018 21:18:11 +0100 Subject: [PATCH] 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 --- .../src/common/downgrade_component_adapter.ts | 58 +++++++++---------- packages/upgrade/test/dynamic/upgrade_spec.ts | 51 +++++++++++++++- .../integration/downgrade_component_spec.ts | 54 ++++++++++++++++- 3 files changed, 131 insertions(+), 32 deletions(-) diff --git a/packages/upgrade/src/common/downgrade_component_adapter.ts b/packages/upgrade/src/common/downgrade_component_adapter.ts index 6ba0238675..b5dfbfef2b 100644 --- a/packages/upgrade/src/common/downgrade_component_adapter.ts +++ b/packages/upgrade/src/common/downgrade_component_adapter.ts @@ -168,42 +168,40 @@ export class DowngradeComponentAdapter { const outputs = this.componentFactory.outputs || []; for (let j = 0; j < outputs.length; j++) { const output = new PropertyBinding(outputs[j].propName, outputs[j].templateName); - let expr: string|null = null; - let assignExpr = false; - const bindonAttr = output.bindonAttr.substring(0, output.bindonAttr.length - 6); const bracketParenAttr = `[(${output.bracketParenAttr.substring(2, output.bracketParenAttr.length - 8)})]`; - + // order below is important - first update bindings then evaluate expressions + if (attrs.hasOwnProperty(bindonAttr)) { + this.subscribeToOutput(output, attrs[bindonAttr], true); + } + if (attrs.hasOwnProperty(bracketParenAttr)) { + this.subscribeToOutput(output, attrs[bracketParenAttr], true); + } if (attrs.hasOwnProperty(output.onAttr)) { - expr = attrs[output.onAttr]; - } else if (attrs.hasOwnProperty(output.parenAttr)) { - expr = attrs[output.parenAttr]; - } else if (attrs.hasOwnProperty(bindonAttr)) { - expr = attrs[bindonAttr]; - assignExpr = true; - } else if (attrs.hasOwnProperty(bracketParenAttr)) { - expr = attrs[bracketParenAttr]; - assignExpr = true; + this.subscribeToOutput(output, attrs[output.onAttr]); } + if (attrs.hasOwnProperty(output.parenAttr)) { + this.subscribeToOutput(output, attrs[output.parenAttr]); + } + } + } - if (expr != null && assignExpr != null) { - const getter = this.$parse(expr); - const setter = getter.assign; - if (assignExpr && !setter) { - throw new Error(`Expression '${expr}' is not assignable!`); - } - const emitter = this.component[output.prop] as EventEmitter; - if (emitter) { - emitter.subscribe({ - next: assignExpr ? (v: any) => setter !(this.scope, v) : - (v: any) => getter(this.scope, {'$event': v}) - }); - } else { - throw new Error( - `Missing emitter '${output.prop}' on component '${getComponentName(this.componentFactory.componentType)}'!`); - } - } + private subscribeToOutput(output: PropertyBinding, expr: string, isAssignment: boolean = false) { + const getter = this.$parse(expr); + const setter = getter.assign; + if (isAssignment && !setter) { + throw new Error(`Expression '${expr}' is not assignable!`); + } + const emitter = this.component[output.prop] as EventEmitter; + if (emitter) { + emitter.subscribe({ + next: isAssignment ? (v: any) => setter !(this.scope, v) : + (v: any) => getter(this.scope, {'$event': v}) + }); + } else { + throw new Error( + `Missing emitter '${output.prop}' on component '${getComponentName(this.componentFactory.componentType)}'!`); } } diff --git a/packages/upgrade/test/dynamic/upgrade_spec.ts b/packages/upgrade/test/dynamic/upgrade_spec.ts index 48d3412834..a3e70858f7 100644 --- a/packages/upgrade/test/dynamic/upgrade_spec.ts +++ b/packages/upgrade/test/dynamic/upgrade_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ChangeDetectorRef, Component, EventEmitter, Input, NO_ERRORS_SCHEMA, NgModule, NgModuleFactory, NgZone, OnChanges, OnDestroy, SimpleChange, SimpleChanges, Testability, destroyPlatform, forwardRef} from '@angular/core'; +import {ChangeDetectorRef, Component, EventEmitter, Input, NO_ERRORS_SCHEMA, NgModule, NgModuleFactory, NgZone, OnChanges, OnDestroy, Output, SimpleChange, SimpleChanges, Testability, destroyPlatform, forwardRef} from '@angular/core'; import {async, fakeAsync, flushMicrotasks, tick} from '@angular/core/testing'; import {BrowserModule} from '@angular/platform-browser'; import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; @@ -434,6 +434,55 @@ withEachNg1Version(() => { })); + it('should support two-way binding and event listener', async(() => { + const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module)); + const listenerSpy = jasmine.createSpy('$rootScope.listener'); + const ng1Module = angular.module('ng1', []).run(($rootScope: angular.IScope) => { + $rootScope['value'] = 'world'; + $rootScope['listener'] = listenerSpy; + }); + + @Component({selector: 'ng2', template: `model: {{model}};`}) + class Ng2Component implements OnChanges { + ngOnChangesCount = 0; + @Input() model = '?'; + @Output() modelChange = new EventEmitter(); + + ngOnChanges(changes: SimpleChanges) { + switch (this.ngOnChangesCount++) { + case 0: + expect(changes.model.currentValue).toBe('world'); + this.modelChange.emit('newC'); + break; + case 1: + expect(changes.model.currentValue).toBe('newC'); + break; + default: + throw new Error('Called too many times! ' + JSON.stringify(changes)); + } + } + } + + ng1Module.directive('ng2', adapter.downgradeNg2Component(Ng2Component)); + + @NgModule({declarations: [Ng2Component], imports: [BrowserModule]}) + class Ng2Module { + ngDoBootstrap() {} + } + + const element = html(` +
+ + | value: {{value}} +
+ `); + adapter.bootstrap(element, ['ng1']).ready((ref) => { + expect(multiTrim(element.textContent)).toEqual('model: newC; | value: newC'); + expect(listenerSpy).toHaveBeenCalledWith('newC'); + ref.dispose(); + }); + })); + it('should initialize inputs in time for `ngOnChanges`', async(() => { const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module)); diff --git a/packages/upgrade/test/static/integration/downgrade_component_spec.ts b/packages/upgrade/test/static/integration/downgrade_component_spec.ts index 2f977c78e8..d6254b7b1e 100644 --- a/packages/upgrade/test/static/integration/downgrade_component_spec.ts +++ b/packages/upgrade/test/static/integration/downgrade_component_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ChangeDetectorRef, Compiler, Component, ComponentFactoryResolver, Directive, ElementRef, EventEmitter, Injector, Input, NgModule, NgModuleRef, OnChanges, OnDestroy, SimpleChanges, destroyPlatform} from '@angular/core'; +import {ChangeDetectorRef, Compiler, Component, ComponentFactoryResolver, Directive, ElementRef, EventEmitter, Injector, Input, NgModule, NgModuleRef, OnChanges, OnDestroy, Output, SimpleChanges, destroyPlatform} from '@angular/core'; import {async, fakeAsync, tick} from '@angular/core/testing'; import {BrowserModule} from '@angular/platform-browser'; import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; @@ -148,6 +148,58 @@ withEachNg1Version(() => { }); })); + it('should support two-way binding and event listener', async(() => { + const listenerSpy = jasmine.createSpy('$rootScope.listener'); + const ng1Module = angular.module('ng1', []).run(($rootScope: angular.IScope) => { + $rootScope['value'] = 'world'; + $rootScope['listener'] = listenerSpy; + }); + + @Component({selector: 'ng2', template: `model: {{model}};`}) + class Ng2Component implements OnChanges { + ngOnChangesCount = 0; + @Input() model = '?'; + @Output() modelChange = new EventEmitter(); + + ngOnChanges(changes: SimpleChanges) { + switch (this.ngOnChangesCount++) { + case 0: + expect(changes.model.currentValue).toBe('world'); + this.modelChange.emit('newC'); + break; + case 1: + expect(changes.model.currentValue).toBe('newC'); + break; + default: + throw new Error('Called too many times! ' + JSON.stringify(changes)); + } + } + } + + ng1Module.directive('ng2', downgradeComponent({component: Ng2Component})); + + @NgModule({ + declarations: [Ng2Component], + entryComponents: [Ng2Component], + imports: [BrowserModule, UpgradeModule] + }) + class Ng2Module { + ngDoBootstrap() {} + } + + const element = html(` +
+ + | value: {{value}} +
+ `); + + bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then((upgrade) => { + expect(multiTrim(element.textContent)).toEqual('model: newC; | value: newC'); + expect(listenerSpy).toHaveBeenCalledWith('newC'); + }); + })); + it('should run change-detection on every digest (by default)', async(() => { let ng2Component: Ng2Component;