diff --git a/lib/common/events.ts b/lib/common/events.ts index e14a38685..bd5791b2a 100644 --- a/lib/common/events.ts +++ b/lib/common/events.ts @@ -45,6 +45,11 @@ export interface PatchEventTargetOptions { export function patchEventTarget( api: _ZonePrivate, _global: any, apis: any[], patchOptions?: PatchEventTargetOptions) { const invokeTask = function(task: any, target: any, event: Event) { + // for better performance, check isRemoved which is set + // by removeEventListener + if (task.isRemoved) { + return; + } const delegate = task.callback; if (typeof delegate === OBJECT_TYPE && delegate.handleEvent) { // create the bind version of handleEvnet when invoke @@ -62,8 +67,16 @@ export function patchEventTarget( const tasks = target[zoneSymbolEventNames[event.type][FALSE_STR]]; if (tasks) { // invoke all tasks which attached to current target with given event.type and capture = false - for (let i = 0; i < tasks.length; i++) { - invokeTask(tasks[i], target, event); + if (tasks.length === 1) { + invokeTask(tasks[0], target, event); + } else { + // https://github.com/angular/zone.js/issues/836 + // copy the tasks array before invoke, to avoid + // the callback will remove itself or other listener + const copyTasks = tasks.slice(); + for (let i = 0; i < copyTasks.length; i++) { + invokeTask(copyTasks[i], target, event); + } } } }; @@ -74,8 +87,17 @@ export function patchEventTarget( const tasks = target[zoneSymbolEventNames[event.type][TRUE_STR]]; if (tasks) { - for (let i = 0; i < tasks.length; i++) { - invokeTask(tasks[i], target, event); + // invoke all tasks which attached to current target with given event.type and capture = false + if (tasks.length === 1) { + invokeTask(tasks[0], target, event); + } else { + // https://github.com/angular/zone.js/issues/836 + // copy the tasks array before invoke, to avoid + // the callback will remove itself or other listener + const copyTasks = tasks.slice(); + for (let i = 0; i < copyTasks.length; i++) { + invokeTask(copyTasks[i], target, event); + } } } }; @@ -169,7 +191,7 @@ export function patchEventTarget( // if all tasks for the eventName + capture have gone, // we will really remove the global event callback, // if not, return - if (!task.remove) { + if (!task.allRemoved) { return; } return nativeRemoveEventListener.apply(task.target, [ @@ -372,10 +394,12 @@ export function patchEventTarget( const typeOfDelegate = typeof delegate; if (compare(existingTask, delegate)) { existingTasks.splice(i, 1); + // set isRemoved to data for faster invokeTask check + (existingTask as any).isRemoved = true; if (existingTasks.length === 0) { // all tasks for the eventName + capture have gone, // remove globalZoneAwareCallback and remove the task cache from target - (existingTask as any).remove = true; + (existingTask as any).allRemoved = true; target[symbolEventName] = null; } existingTask.zone.cancelTask(existingTask); diff --git a/test/browser/browser.spec.ts b/test/browser/browser.spec.ts index c5351bf61..eb53c4529 100644 --- a/test/browser/browser.spec.ts +++ b/test/browser/browser.spec.ts @@ -781,6 +781,391 @@ describe('Zone', function() { }); }); + describe('should be able to remove eventListener during eventListener callback', function() { + it('should be able to remove first eventListener during eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + button.removeEventListener('click', listener1); + logs.push('listener1'); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(3); + expect(logs).toEqual(['listener1', 'listener2', 'listener3']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(2); + expect(logs).toEqual(['listener2', 'listener3']); + + button.removeEventListener('click', listener2); + button.removeEventListener('click', listener3); + }); + + it('should be able to remove middle eventListener during eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + button.removeEventListener('click', listener2); + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(3); + expect(logs).toEqual(['listener1', 'listener2', 'listener3']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(2); + expect(logs).toEqual(['listener1', 'listener3']); + + button.removeEventListener('click', listener1); + button.removeEventListener('click', listener3); + }); + + it('should be able to remove last eventListener during eventListener callback', function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + button.removeEventListener('click', listener3); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(3); + expect(logs).toEqual(['listener1', 'listener2', 'listener3']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(2); + expect(logs).toEqual(['listener1', 'listener2']); + + button.removeEventListener('click', listener1); + button.removeEventListener('click', listener2); + }); + + it('should be able to remove all afterward eventListener during eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + button.removeEventListener('click', listener2); + button.removeEventListener('click', listener3); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(1); + expect(logs).toEqual(['listener1']); + + button.removeEventListener('click', listener1); + }); + + it('should be able to remove part of afterward eventListener during eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + button.removeEventListener('click', listener2); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(2); + expect(logs).toEqual(['listener1', 'listener3']); + + button.removeEventListener('click', listener1); + button.removeEventListener('click', listener3); + }); + + it('should be able to remove all beforeward and afterward eventListener during eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + logs.push('listener2'); + button.removeEventListener('click', listener1); + button.removeEventListener('click', listener3); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(2); + expect(logs).toEqual(['listener1', 'listener2']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(1); + expect(logs).toEqual(['listener2']); + + button.removeEventListener('click', listener2); + }); + + it('should be able to remove part of beforeward and afterward eventListener during eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + button.removeEventListener('click', listener2); + button.removeEventListener('click', listener4); + } + }; + const listener4 = function() { + logs.push('listener4'); + }; + const listener5 = function() { + logs.push('listener5'); + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + button.addEventListener('click', listener4); + button.addEventListener('click', listener5); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(4); + expect(logs).toEqual(['listener1', 'listener2', 'listener3', 'listener5']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(3); + expect(logs).toEqual(['listener1', 'listener3', 'listener5']); + + button.removeEventListener('click', listener1); + button.removeEventListener('click', listener3); + button.removeEventListener('click', listener5); + }); + + it('should be able to remove all beforeward eventListener during eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + button.removeEventListener('click', listener1); + button.removeEventListener('click', listener2); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(3); + expect(logs).toEqual(['listener1', 'listener2', 'listener3']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(1); + expect(logs).toEqual(['listener3']); + + button.removeEventListener('click', listener3); + }); + + it('should be able to remove part of beforeward eventListener during eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + button.removeEventListener('click', listener1); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(3); + expect(logs).toEqual(['listener1', 'listener2', 'listener3']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(2); + expect(logs).toEqual(['listener2', 'listener3']); + + button.removeEventListener('click', listener2); + button.removeEventListener('click', listener3); + }); + + it('should be able to remove all eventListeners during first eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + (button as any).removeAllListeners('click'); + logs.push('listener1'); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(1); + expect(logs).toEqual(['listener1']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(0); + }); + + it('should be able to remove all eventListeners during middle eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + (button as any).removeAllListeners('click'); + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(2); + expect(logs).toEqual(['listener1', 'listener2']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(0); + }); + + it('should be able to remove all eventListeners during last eventListener callback', + function() { + let logs: string[] = []; + const listener1 = function() { + logs.push('listener1'); + }; + const listener2 = function() { + logs.push('listener2'); + }; + const listener3 = { + handleEvent: function(event: Event) { + logs.push('listener3'); + (button as any).removeAllListeners('click'); + } + }; + + button.addEventListener('click', listener1); + button.addEventListener('click', listener2); + button.addEventListener('click', listener3); + + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(3); + expect(logs).toEqual(['listener1', 'listener2', 'listener3']); + + logs = []; + button.dispatchEvent(clickEvent); + expect(logs.length).toBe(0); + }); + }); + it('should be able to get eventListeners of specified event form EventTarget', function() { const listener1 = function() {}; const listener2 = function() {};