From 59d44f0a1c166b1dd20b34a511b6aa9c547705c7 Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Sun, 19 Mar 2023 11:20:59 +0000 Subject: [PATCH] fix(zone.js): should allow add passive/non-passive listeners together Close #45020 In the current version, if we add both `passive` and `not passive` listeners for the same eventName together, they will be registered with all passive or all non passive listeners depends on the order. ``` import 'zone.js'; div1.addEventListener('mousemove', (ev) => {}, { passive: true }); div1.addEventListener('mousemove', (ev) => { ev.preventDefault(); // throws error since this one is also be registered as a passive event handler }); div2.addEventListener('mousemove', (ev) => { }); div2.addEventListener('mousemove', (ev) => { ev.preventDefault(); // will not throw error since this one is also be registered as non passive event handler }, { passive: true }); ``` So this PR fix this issue and allow both passive and non-passive listeners registeration together whatever the order. --- packages/zone.js/lib/common/events.ts | 32 +++++++- packages/zone.js/test/browser/browser.spec.ts | 80 ++++++++++++++++++- 2 files changed, 108 insertions(+), 4 deletions(-) diff --git a/packages/zone.js/lib/common/events.ts b/packages/zone.js/lib/common/events.ts index 58f60f77ed115d..c2dd480f068739 100644 --- a/packages/zone.js/lib/common/events.ts +++ b/packages/zone.js/lib/common/events.ts @@ -104,12 +104,25 @@ export function patchEventTarget( const PREPEND_EVENT_LISTENER = 'prependListener'; const PREPEND_EVENT_LISTENER_SOURCE = '.' + PREPEND_EVENT_LISTENER + ':'; + function isPassiveEventListener(options: any): boolean { + return typeof options === 'object' && options?.passive === true; + } + const invokeTask = function(task: any, target: any, event: Event): Error|undefined { // for better performance, check isRemoved which is set // by removeEventListener if (task.isRemoved) { return; } + const options = task.options; + // If the listener is a pssive event listener, we don't execute + // it, since the passive listener is registered separately. + // We only keep the passive listener in the target, so we can + // call target.eventListeners() API to retrive all event listeners + // for both passive and non-passive ones easily. + if (isPassiveEventListener(options)) { + return; + } const delegate = task.callback; if (typeof delegate === 'object' && delegate.handleEvent) { // create the bind version of handleEvent when invoke @@ -126,7 +139,6 @@ export function patchEventTarget( } catch (err: any) { error = err; } - const options = task.options; if (options && typeof options === 'object' && options.once) { // if options.once is true, after invoke once remove listener here // only browser need to do this, nodejs eventEmitter will cal removeListener @@ -271,13 +283,21 @@ export function patchEventTarget( if (!options) { return {passive: true}; } - if (typeof options === 'object' && options.passive !== false) { + if (isPassiveEventListener(options)) { return {...options, passive: true}; } return options; } const customScheduleGlobal = function(task: Task) { + // https://github.com/angular/angular/issues/45020 + // For passive event listener, we need to addEventListener with the listener callback + // instead of the globalCallback. Otherwise, all listeners with the same eventName + // will be registered as all passive or all non passive. + if (isPassiveEventListener(taskData.options)) { + return nativeAddEventListener.call( + taskData.target, taskData.eventName, task.invoke, taskData.options); + } // if there is already a task for the eventName + capture, // just return, because we use the shared globalZoneAwareCallback here. if (taskData.isExisting) { @@ -410,6 +430,7 @@ export function patchEventTarget( const capture = !options ? false : typeof options === 'boolean' ? true : options.capture; const once = options && typeof options === 'object' ? options.once : false; + const isPassive = isPassiveEventListener(options); const zone = Zone.current; let symbolEventNames = zoneSymbolEventNames[eventName]; @@ -434,6 +455,11 @@ export function patchEventTarget( } else { existingTasks = target[symbolEventName] = []; } + // check whether all tasks are passive. + if (passiveSupported && existingTasks.length && + !existingTasks.some((t: any) => !isPassiveEventListener(t.options))) { + isExisting = false; + } let source; const constructorName = target.constructor['name']; const targetSource = globalSources[constructorName]; @@ -458,7 +484,7 @@ export function patchEventTarget( taskData.eventName = eventName; taskData.isExisting = isExisting; - const data = useGlobalCallback ? OPTIMIZED_ZONE_EVENT_TASK_DATA : undefined; + const data = !isPassive && useGlobalCallback ? OPTIMIZED_ZONE_EVENT_TASK_DATA : undefined; // keep taskData into data to allow onScheduleEventTask to access the task information if (data) { diff --git a/packages/zone.js/test/browser/browser.spec.ts b/packages/zone.js/test/browser/browser.spec.ts index 39c60e9229ae7c..190b90d8cc62ad 100644 --- a/packages/zone.js/test/browser/browser.spec.ts +++ b/packages/zone.js/test/browser/browser.spec.ts @@ -1505,6 +1505,84 @@ describe('Zone', function() { button.removeEventListener('click', listener); })); + it('should support addEventListener passive first and non passive after', + ifEnvSupports(supportEventListenerOptions, function() { + const hookSpy = jasmine.createSpy('hook'); + const logs: string[] = []; + const zone = rootZone.fork({ + name: 'spy', + onScheduleTask: ( + parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, task: Task): + any => { + hookSpy(); + return parentZoneDelegate.scheduleTask(targetZone, task); + } + }); + + const listener = (e: Event) => { + logs.push(e.defaultPrevented.toString()); + e.preventDefault(); + logs.push(e.defaultPrevented.toString()); + }; + + const listener1 = (e: Event) => { + logs.push(e.defaultPrevented.toString()); + e.preventDefault(); + logs.push(e.defaultPrevented.toString()); + }; + + zone.run(function() { + (button as any).addEventListener('click', listener, {passive: true}); + (button as any).addEventListener('click', listener1); + }); + + button.dispatchEvent(clickEvent); + + expect(hookSpy).toHaveBeenCalled(); + expect(logs).toEqual(['false', 'false', 'false', 'true']); + + button.removeEventListener('click', listener); + })); + + it('should support addEventListener non passive first and passive after', + ifEnvSupports(supportEventListenerOptions, function() { + const hookSpy = jasmine.createSpy('hook'); + const logs: string[] = []; + const zone = rootZone.fork({ + name: 'spy', + onScheduleTask: ( + parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, task: Task): + any => { + hookSpy(); + return parentZoneDelegate.scheduleTask(targetZone, task); + } + }); + + const listener = (e: Event) => { + logs.push(e.defaultPrevented.toString()); + e.preventDefault(); + logs.push(e.defaultPrevented.toString()); + }; + + const listener1 = (e: Event) => { + logs.push(e.defaultPrevented.toString()); + e.preventDefault(); + logs.push(e.defaultPrevented.toString()); + }; + + zone.run(function() { + (button as any).addEventListener('click', listener); + (button as any).addEventListener('click', listener1, {passive: true}); + }); + + button.dispatchEvent(clickEvent); + + expect(hookSpy).toHaveBeenCalled(); + expect(logs).toEqual(['false', 'true', 'true', 'true']); + + button.removeEventListener('click', listener); + })); + describe('passiveEvents by global settings', () => { let logs: string[] = []; const listener = (e: Event) => { @@ -1538,7 +1616,7 @@ describe('Zone', function() { }); it('should be passive with global variable defined even without passive options and with capture', () => { - testPassive('touchstart', 'default will run', {capture: true}); + testPassive('touchstart', 'defaultPrevented', {capture: true}); }); it('should be passive with global variable defined with capture option', () => { testPassive('touchstart', 'default will run', true);