From 2823de95ac3fd8a50f163cf07c197ba53e473f7a Mon Sep 17 00:00:00 2001 From: Andrey Ovchinnikov Date: Wed, 4 Apr 2018 14:50:19 +0300 Subject: [PATCH 1/4] Event with subscriptions should be fired inside NgZone --- src/core/component.ts | 24 ++++++++++++---- src/core/events-strategy.ts | 56 ++++++++++++++++++++++--------------- tests/src/ui/list.spec.ts | 4 +++ 3 files changed, 55 insertions(+), 29 deletions(-) diff --git a/src/core/component.ts b/src/core/component.ts index cd90a8e61..f0502adc5 100644 --- a/src/core/component.ts +++ b/src/core/component.ts @@ -20,7 +20,7 @@ import { TransferState, makeStateKey } from '@angular/platform-browser'; import { DxTemplateDirective } from './template'; import { IDxTemplateHost, DxTemplateHost } from './template-host'; -import { EmitterHelper } from './events-strategy'; +import { EmitterHelper, NgEventsStrategy } from './events-strategy'; import { WatcherHelper } from './watcher-helper'; import * as events from 'devextreme/events'; import { @@ -63,7 +63,6 @@ export abstract class DxComponent implements OnChanges, OnInit, DoCheck, AfterCo }); } private _initOptions() { - this._initialOptions.eventsStrategy = this.eventHelper.strategy; this._initialOptions.integrationOptions.watchMethod = this.watcherHelper.getWatchMethod(); } @@ -76,9 +75,22 @@ export abstract class DxComponent implements OnChanges, OnInit, DoCheck, AfterCo } protected _createEventEmitters(events) { - events.forEach(event => { - this.eventHelper.createEmitter(event.emit, event.subscribe); - }); + let ngZone = this.ngZone; + this.eventHelper.createEmitters(events); + + this._initialOptions = { + eventsStrategy: (instance) => { + let strategy = new NgEventsStrategy(this.ngZone, instance); + this.eventHelper.setStrategy(strategy); + return strategy; + }, + nestedComponentOptions: function(component) { + return { + eventsStrategy: (instance) => { return new NgEventsStrategy(ngZone, instance); }, + nestedComponentOptions: component.option('nestedComponentOptions') + }; + } + }; } _shouldOptionChange(name: string, value: any) { if (this.changedOptions.hasOwnProperty(name)) { @@ -156,7 +168,7 @@ export abstract class DxComponent implements OnChanges, OnInit, DoCheck, AfterCo this.templates = []; templateHost.setHost(this); this._collectionContainerImpl = new CollectionNestedOptionContainerImpl(this._setOption.bind(this)); - this.eventHelper = new EmitterHelper(this.ngZone, this); + this.eventHelper = new EmitterHelper(this); } ngOnChanges(changes: SimpleChanges) { diff --git a/src/core/events-strategy.ts b/src/core/events-strategy.ts index 234c31cef..7d8e066bd 100644 --- a/src/core/events-strategy.ts +++ b/src/core/events-strategy.ts @@ -1,33 +1,32 @@ import { EventEmitter, NgZone } from '@angular/core'; import { DxComponent } from './component'; -const dxToNgEventNames = {}; - interface IEventSubscription { handler: any; unsubscribe: () => void; } - export class NgEventsStrategy { private subscriptions: { [key: string]: IEventSubscription[] } = {}; + private events: { [key: string]: EventEmitter } = {}; - constructor(private component: DxComponent, private ngZone: NgZone) { } + constructor(private ngZone: NgZone, private instance: any) { } hasEvent(name: string) { - return this.ngZone.run(() => { - return this.getEmitter(name).observers.length; - }); + return this.getEmitter(name).observers.length; } fireEvent(name, args) { - this.ngZone.run(() => { - this.getEmitter(name).next(args && args[0]); - }); + let emitter = this.getEmitter(name); + if (emitter.observers.length) { + this.ngZone.run(() => { + this.getEmitter(name).next(args && args[0]); + }); + } } on(name, handler) { let eventSubscriptions = this.subscriptions[name] || [], - subcription = this.getEmitter(name).subscribe(handler.bind(this.component.instance)), + subcription = this.getEmitter(name).subscribe(handler.bind(this.instance)), unsubscribe = subcription.unsubscribe.bind(subcription); eventSubscriptions.push({ handler, unsubscribe }); @@ -55,22 +54,25 @@ export class NgEventsStrategy { dispose() {} + public addEmitter(eventName: string, emitter: EventEmitter) { + this.events[eventName] = emitter; + } + private getEmitter(eventName: string): EventEmitter { - let ngEventName = dxToNgEventNames[eventName]; - if (!this.component[ngEventName]) { - this.component[ngEventName] = new EventEmitter(); + if (!this.events[eventName]) { + this.events[eventName] = new EventEmitter(); } - return this.component[ngEventName]; + return this.events[eventName]; } } export class EmitterHelper { strategy: NgEventsStrategy; lockedValueChangeEvent = false; + events: any[]; + + constructor(private component: DxComponent) { } - constructor(ngZone: NgZone, public component: DxComponent) { - this.strategy = new NgEventsStrategy(component, ngZone); - } fireNgEvent(eventName: string, eventArgs: any) { if (this.lockedValueChangeEvent && eventName === 'valueChange') { return; @@ -80,11 +82,19 @@ export class EmitterHelper { emitter.next(eventArgs && eventArgs[0]); } } - createEmitter(ngEventName: string, dxEventName: string) { - this.component[ngEventName] = new EventEmitter(); - if (dxEventName) { - dxToNgEventNames[dxEventName] = ngEventName; - } + + createEmitters(events: any[]) { + this.events = events; + events.forEach(event => { + this.component[event.emit] = new EventEmitter(); + }); + } + + public setStrategy(value: NgEventsStrategy) { + this.strategy = value; + this.events.filter(event => event.subscribe).forEach(event => { + this.strategy.addEmitter(event.subscribe, this.component[event.emit]); + }); } } diff --git a/tests/src/ui/list.spec.ts b/tests/src/ui/list.spec.ts index 3eae0455e..bcfbd2202 100644 --- a/tests/src/ui/list.spec.ts +++ b/tests/src/ui/list.spec.ts @@ -565,6 +565,10 @@ describe('DxList', () => { expect(onChangesSpy.calls.count()).toBe(0); instance.element().dispatchEvent(new Event('mouseover')); + expect(onChangesSpy.calls.count()).toBe(0); + let item = instance.element().querySelector('.dx-item'); + item.click(); + expect(onChangesSpy.calls.count()).toBe(0); fixture.autoDetectChanges(false); }); From 99608b52795405da09a39e5ccc7921bc7180b833 Mon Sep 17 00:00:00 2001 From: "ovchinnikov.andrey" Date: Thu, 12 Apr 2018 11:17:57 +0300 Subject: [PATCH 2/4] Update DevExtreme --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 845dfbd86..e54e6701d 100644 --- a/package.json +++ b/package.json @@ -17,13 +17,13 @@ "author": "Developer Express Inc.", "license": "MIT", "peerDependencies": { - "devextreme": "~18.1.1-pre-18093", + "devextreme": "~18.1.1-pre-18101", "@angular/core": ">5.0.0", "@angular/common": ">5.0.0", "@angular/forms": ">5.0.0" }, "devDependencies": { - "devextreme": "~18.1.1-pre-18093", + "devextreme": "~18.1.1-pre-18101", "zone.js": "^0.8.25", "@angular/animations": "^5.0.0", "@angular/common": "^5.0.0", From 614f90eb34bd8f0e2a2bdc4001db9d0ca2bb8420 Mon Sep 17 00:00:00 2001 From: "ovchinnikov.andrey" Date: Thu, 12 Apr 2018 15:02:25 +0300 Subject: [PATCH 3/4] Refactoring --- src/core/component.ts | 27 +++++++++++++++------------ src/core/events-strategy.ts | 10 ---------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/core/component.ts b/src/core/component.ts index f0502adc5..749c1df41 100644 --- a/src/core/component.ts +++ b/src/core/component.ts @@ -78,18 +78,21 @@ export abstract class DxComponent implements OnChanges, OnInit, DoCheck, AfterCo let ngZone = this.ngZone; this.eventHelper.createEmitters(events); - this._initialOptions = { - eventsStrategy: (instance) => { - let strategy = new NgEventsStrategy(this.ngZone, instance); - this.eventHelper.setStrategy(strategy); - return strategy; - }, - nestedComponentOptions: function(component) { - return { - eventsStrategy: (instance) => { return new NgEventsStrategy(ngZone, instance); }, - nestedComponentOptions: component.option('nestedComponentOptions') - }; - } + this._initialOptions.eventsStrategy = (instance) => { + let strategy = new NgEventsStrategy(ngZone, instance); + + events.filter(event => event.subscribe).forEach(event => { + strategy.addEmitter(event.subscribe, this[event.emit]); + }); + + return strategy; + }; + + this._initialOptions.nestedComponentOptions = function(component) { + return { + eventsStrategy: (instance) => { return new NgEventsStrategy(ngZone, instance); }, + nestedComponentOptions: component.option('nestedComponentOptions') + }; }; } _shouldOptionChange(name: string, value: any) { diff --git a/src/core/events-strategy.ts b/src/core/events-strategy.ts index 6e28bab2d..81fd4e141 100644 --- a/src/core/events-strategy.ts +++ b/src/core/events-strategy.ts @@ -67,8 +67,6 @@ export class NgEventsStrategy { } export class EmitterHelper { - strategy: NgEventsStrategy; - events: any[]; constructor(private component: DxComponent) { } @@ -80,17 +78,9 @@ export class EmitterHelper { } createEmitters(events: any[]) { - this.events = events; events.forEach(event => { this.component[event.emit] = new EventEmitter(); }); } - - public setStrategy(value: NgEventsStrategy) { - this.strategy = value; - this.events.filter(event => event.subscribe).forEach(event => { - this.strategy.addEmitter(event.subscribe, this.component[event.emit]); - }); - } } From a0fc23529646c438b24b2da24c0bee2e20ba12b7 Mon Sep 17 00:00:00 2001 From: "ovchinnikov.andrey" Date: Fri, 13 Apr 2018 10:53:23 +0300 Subject: [PATCH 4/4] Refactoring --- src/core/events-strategy.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/core/events-strategy.ts b/src/core/events-strategy.ts index 81fd4e141..1fb416f9f 100644 --- a/src/core/events-strategy.ts +++ b/src/core/events-strategy.ts @@ -18,9 +18,7 @@ export class NgEventsStrategy { fireEvent(name, args) { let emitter = this.getEmitter(name); if (emitter.observers.length) { - this.ngZone.run(() => { - this.getEmitter(name).next(args && args[0]); - }); + this.ngZone.run(() => emitter.next(args && args[0])); } }