diff --git a/packages/core/test/view/component_view_spec.ts b/packages/core/test/view/component_view_spec.ts index 017669cad1..11adf801fe 100644 --- a/packages/core/test/view/component_view_spec.ts +++ b/packages/core/test/view/component_view_spec.ts @@ -10,7 +10,8 @@ import {Injector, RenderComponentType, RootRenderer, Sanitizer, SecurityContext, import {ArgumentType, BindingFlags, NodeCheckFn, NodeDef, NodeFlags, OutputType, RootData, Services, ViewData, ViewDefinition, ViewFlags, ViewHandleEventFn, ViewState, ViewUpdateFn, anchorDef, asElementData, asProviderData, directiveDef, elementDef, rootRenderNodes, textDef, viewDef} from '@angular/core/src/view/index'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; -import {createRootView, isBrowser, recordNodeToRemove} from './helper'; +import {callMostRecentEventListenerHandler, createRootView, isBrowser, recordNodeToRemove} from './helper'; + /** * We map addEventListener to the Zones internal name. This is because we want to be fast @@ -224,7 +225,7 @@ export function main() { expect(update).not.toHaveBeenCalled(); // auto attach on events - addListenerSpy.calls.mostRecent().args[1]('SomeEvent'); + callMostRecentEventListenerHandler(addListenerSpy, 'SomeEvent'); update.calls.reset(); Services.checkAndUpdateView(view); expect(update).toHaveBeenCalled(); diff --git a/packages/core/test/view/element_spec.ts b/packages/core/test/view/element_spec.ts index dbd3999e16..88932b6bcb 100644 --- a/packages/core/test/view/element_spec.ts +++ b/packages/core/test/view/element_spec.ts @@ -12,7 +12,8 @@ import {ArgumentType, BindingFlags, DebugContext, NodeDef, NodeFlags, OutputType import {TestBed} from '@angular/core/testing'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; -import {ARG_TYPE_VALUES, checkNodeInlineOrDynamic, createRootView, isBrowser, recordNodeToRemove} from './helper'; +import {ARG_TYPE_VALUES, callMostRecentEventListenerHandler, checkNodeInlineOrDynamic, createRootView, isBrowser, recordNodeToRemove} from './helper'; + /** * We map addEventListener to the Zones internal name. This is because we want to be fast @@ -225,7 +226,7 @@ export function main() { expect(addListenerSpy).toHaveBeenCalled(); expect(addListenerSpy.calls.mostRecent().args[0]).toBe('windowClick'); - addListenerSpy.calls.mostRecent().args[1]({name: 'windowClick'}); + callMostRecentEventListenerHandler(addListenerSpy, {name: 'windowClick'}); expect(handleEventSpy).toHaveBeenCalled(); const handleEventArgs = handleEventSpy.calls.mostRecent().args; @@ -248,7 +249,7 @@ export function main() { expect(addListenerSpy).toHaveBeenCalled(); expect(addListenerSpy.calls.mostRecent().args[0]).toBe('documentClick'); - addListenerSpy.calls.mostRecent().args[1]({name: 'documentClick'}); + callMostRecentEventListenerHandler(addListenerSpy, {name: 'windowClick'}); expect(handleEventSpy).toHaveBeenCalled(); const handleEventArgs = handleEventSpy.calls.mostRecent().args; @@ -296,7 +297,7 @@ export function main() { NodeFlags.None, null !, null !, 0, 'button', null !, null !, [[null !, 'click']], () => { throw new Error('Test'); })])); - addListenerSpy.calls.mostRecent().args[1]('SomeEvent'); + callMostRecentEventListenerHandler(addListenerSpy, 'SomeEvent'); const err = handleErrorSpy.calls.mostRecent().args[0]; expect(err).toBeTruthy(); expect(err.message).toBe('Test'); diff --git a/packages/core/test/view/helper.ts b/packages/core/test/view/helper.ts index cf6888f8bb..0f26894f6d 100644 --- a/packages/core/test/view/helper.ts +++ b/packages/core/test/view/helper.ts @@ -48,4 +48,19 @@ afterEach(() => { removeNodes.forEach((node) => getDOM().remove(node)); }); export function recordNodeToRemove(node: Node) { removeNodes.push(node); +} + +export function callMostRecentEventListenerHandler(spy: any, params: any) { + const mostRecent = spy.calls.mostRecent(); + if (!mostRecent) { + return; + } + + const obj = mostRecent.object; + const args = mostRecent.args; + + const eventName = args[0]; + const handler = args[1]; + + handler && handler.apply(obj, [{type: eventName}]); } \ No newline at end of file diff --git a/packages/platform-browser/src/dom/events/dom_events.ts b/packages/platform-browser/src/dom/events/dom_events.ts index 3174a91dc1..77a3d68401 100644 --- a/packages/platform-browser/src/dom/events/dom_events.ts +++ b/packages/platform-browser/src/dom/events/dom_events.ts @@ -13,7 +13,8 @@ import {DOCUMENT} from '../dom_tokens'; import {EventManagerPlugin} from './event_manager'; /** - * Detect if Zone is present. If it is then bypass 'addEventListener' since Angular can do much more + * Detect if Zone is present. If it is then use simple zone aware 'addEventListener' + * since Angular can do much more * efficient bookkeeping than Zone can, because we have additional information. This speeds up * addEventListener by 3x. */ @@ -24,6 +25,40 @@ const __symbol__ = Zone && Zone['__symbol__'] || function(v: T): T { const ADD_EVENT_LISTENER: 'addEventListener' = __symbol__('addEventListener'); const REMOVE_EVENT_LISTENER: 'removeEventListener' = __symbol__('removeEventListener'); +const symbolNames: {[key: string]: string} = {}; + +const FALSE = 'FALSE'; +const ANGULAR = 'ANGULAR'; +const NATIVE_ADD_LISTENER = 'addEventListener'; +const NATIVE_REMOVE_LISTENER = 'removeEventListener'; + +interface TaskData { + zone: any; + handler: Function; +} + +// a global listener to handle all dom event, +// so we do not need to create a closure everytime +const globalListener = function(event: Event) { + const symbolName = symbolNames[event.type]; + if (!symbolName) { + return; + } + const taskDatas: TaskData[] = this[symbolName]; + if (!taskDatas) { + return; + } + const args: any = [event]; + taskDatas.forEach(taskData => { + if (taskData.zone !== Zone.current) { + // only use Zone.run when Zone.current not equals to stored zone + return taskData.zone.run(taskData.handler, this, args); + } else { + return taskData.handler.apply(this, args); + } + }); +}; + @Injectable() export class DomEventsPlugin extends EventManagerPlugin { constructor(@Inject(DOCUMENT) doc: any, private ngZone: NgZone) { super(doc); } @@ -37,23 +72,65 @@ export class DomEventsPlugin extends EventManagerPlugin { * This code is about to add a listener to the DOM. If Zone.js is present, than * `addEventListener` has been patched. The patched code adds overhead in both * memory and speed (3x slower) than native. For this reason if we detect that - * Zone.js is present we bypass zone and use native addEventListener instead. - * The result is faster registration but the zone will not be restored. We do - * manual zone restoration in element.ts renderEventHandlerClosure method. + * Zone.js is present we use a simple version of zone aware addEventListener instead. + * The result is faster registration and the zone will be restored. + * But ZoneSpec.onScheduleTask, ZoneSpec.onInvokeTask, ZoneSpec.onCancelTask + * will not be invoked + * We also do manual zone restoration in element.ts renderEventHandlerClosure method. * * NOTE: it is possible that the element is from different iframe, and so we * have to check before we execute the method. */ const self = this; - let byPassZoneJS = element[ADD_EVENT_LISTENER]; + const zoneJsLoaded = element[ADD_EVENT_LISTENER]; let callback: EventListener = handler as EventListener; - if (byPassZoneJS) { - callback = function() { - return self.ngZone.runTask(handler as any, null, arguments as any, eventName); - }; + // if zonejs is loaded and current zone is not ngZone + // we keep Zone.current on target for later restoration. + if (zoneJsLoaded && !NgZone.isInAngularZone()) { + let symbolName = symbolNames[eventName]; + if (!symbolName) { + symbolName = symbolNames[eventName] = __symbol__(ANGULAR + eventName + FALSE); + } + let taskDatas: TaskData[] = (element as any)[symbolName]; + const listenerRegistered = taskDatas && taskDatas.length > 0; + if (!taskDatas) { + taskDatas = (element as any)[symbolName] = []; + } + if (taskDatas.filter(taskData => taskData.handler === callback).length === 0) { + taskDatas.push({zone: Zone.current, handler: callback}); + } + if (!listenerRegistered) { + element[ADD_EVENT_LISTENER](eventName, globalListener, false); + } + } else { + element[NATIVE_ADD_LISTENER](eventName, callback, false); + } + return () => this.removeEventListener(element, eventName, callback); + } + + removeEventListener(target: any, eventName: string, callback: Function): void { + let underlyingRemove = target[REMOVE_EVENT_LISTENER]; + // zone.js not loaded, use native removeEventListener + if (!underlyingRemove) { + return target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]); + } + let symbolName = symbolNames[eventName]; + let taskDatas: TaskData[] = symbolName && target[symbolName]; + if (!taskDatas) { + // addEventListener not using patched version + // just call native removeEventListener + return target[NATIVE_REMOVE_LISTENER].apply(target, [eventName, callback, false]); + } + for (let i = 0; i < taskDatas.length; i++) { + // remove listener from taskDatas if the callback equals + if (taskDatas[i].handler === callback) { + taskDatas.splice(i, 1); + break; + } + } + if (taskDatas.length === 0) { + // all listeners are removed, we can remove the globalListener from target + underlyingRemove.apply(target, [eventName, globalListener, false]); } - element[byPassZoneJS ? ADD_EVENT_LISTENER : 'addEventListener'](eventName, callback, false); - return () => element[byPassZoneJS ? REMOVE_EVENT_LISTENER : 'removeEventListener']( - eventName, callback as any, false); } } diff --git a/packages/platform-browser/test/dom/events/event_manager_spec.ts b/packages/platform-browser/test/dom/events/event_manager_spec.ts index 638a70d918..81ca4705ff 100644 --- a/packages/platform-browser/test/dom/events/event_manager_spec.ts +++ b/packages/platform-browser/test/dom/events/event_manager_spec.ts @@ -90,6 +90,32 @@ export function main() { getDOM().dispatchEvent(element, dispatchedEvent); expect(receivedEvent).toBe(null); }); + + it('should keep zone when addEventListener', () => { + const Zone = (window as any)['Zone']; + + const element = el('
'); + getDOM().appendChild(doc.body, element); + const dispatchedEvent = getDOM().createMouseEvent('click'); + let receivedEvent: any /** TODO #9100 */ = null; + let receivedZone: any = null; + const handler = (e: any /** TODO #9100 */) => { + receivedEvent = e; + receivedZone = Zone.current; + }; + const manager = new EventManager([domEventPlugin], new FakeNgZone()); + + let remover = null; + Zone.root.run(() => { remover = manager.addEventListener(element, 'click', handler); }); + getDOM().dispatchEvent(element, dispatchedEvent); + expect(receivedEvent).toBe(dispatchedEvent); + expect(receivedZone.name).toBe(Zone.root.name); + + receivedEvent = null; + remover && remover(); + getDOM().dispatchEvent(element, dispatchedEvent); + expect(receivedEvent).toBe(null); + }); }); }