From 6860a6ec95ba5ec42f047058fdd7f9d46bef38e1 Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Fri, 15 Dec 2023 05:13:15 +0000 Subject: [PATCH] fix(zone.js): should allow add passive/non-passive listeners together 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. PR closes #45020 --- packages/zone.js/lib/common/events.ts | 66 +++++++++------ packages/zone.js/test/browser/browser.spec.ts | 83 +++++++++++++++++++ 2 files changed, 124 insertions(+), 25 deletions(-) diff --git a/packages/zone.js/lib/common/events.ts b/packages/zone.js/lib/common/events.ts index 0c27767e785a3..66adf578fc7db 100644 --- a/packages/zone.js/lib/common/events.ts +++ b/packages/zone.js/lib/common/events.ts @@ -391,9 +391,8 @@ export function patchEventTarget( return; } - const passive = - passiveSupported && !!passiveEvents && passiveEvents.indexOf(eventName) !== -1; - const options = buildEventListenerOptions(arguments[2], passive); + const isPassiveEvent = passiveSupported && passiveEvents?.includes(eventName); + const options = buildEventListenerOptions(arguments[2], isPassiveEvent); const signal = options && typeof options === 'object' && options.signal && typeof options.signal === 'object' ? options.signal : @@ -402,6 +401,19 @@ export function patchEventTarget( // the signal is an aborted one, just return without attaching the event listener. return; } + const passive = typeof options === 'object' && options?.passive; + + const zone = Zone.current; + let source; + const constructorName = target.constructor['name']; + const targetSource = globalSources[constructorName]; + if (targetSource) { + source = targetSource[eventName]; + } + if (!source) { + source = constructorName + addSource + + (eventNameToString ? eventNameToString(eventName) : eventName); + } if (unpatchedEvents) { // check unpatched list @@ -416,10 +428,22 @@ export function patchEventTarget( } } + if (passive) { + zone.scheduleEventTask( + source, delegate, undefined, + (t) => { + nativeListener.call(target, eventName, t.invoke, options); + }, + (t) => { + nativeRemoveEventListener.call(target, eventName, t.invoke, options); + }); + return; + } + + const capture = !options ? false : typeof options === 'boolean' ? true : options.capture; const once = options && typeof options === 'object' ? options.once : false; - const zone = Zone.current; let symbolEventNames = zoneSymbolEventNames[eventName]; if (!symbolEventNames) { prepareEventNames(eventName, eventNameToString); @@ -428,29 +452,21 @@ export function patchEventTarget( const symbolEventName = symbolEventNames[capture ? TRUE_STR : FALSE_STR]; let existingTasks = target[symbolEventName]; let isExisting = false; - if (existingTasks) { - // already have task registered - isExisting = true; - if (checkDuplicate) { - for (let i = 0; i < existingTasks.length; i++) { - if (compare(existingTasks[i], delegate)) { - // same callback, same capture, same event name, just return - return; + if (!passive) { + if (existingTasks) { + // already have task registered + isExisting = true; + if (checkDuplicate) { + for (let i = 0; i < existingTasks.length; i++) { + if (compare(existingTasks[i], delegate)) { + // same callback, same capture, same event name, just return + return; + } } } + } else { + existingTasks = target[symbolEventName] = []; } - } else { - existingTasks = target[symbolEventName] = []; - } - let source; - const constructorName = target.constructor['name']; - const targetSource = globalSources[constructorName]; - if (targetSource) { - source = targetSource[eventName]; - } - if (!source) { - source = constructorName + addSource + - (eventNameToString ? eventNameToString(eventName) : eventName); } // do not create a new object as task.data to pass those things // just use the global shared one @@ -466,7 +482,7 @@ export function patchEventTarget( taskData.eventName = eventName; taskData.isExisting = isExisting; - const data = useGlobalCallback ? OPTIMIZED_ZONE_EVENT_TASK_DATA : undefined; + const data = useGlobalCallback && !passive ? 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 974d2d4ce44fc..10c09398d325e 100644 --- a/packages/zone.js/test/browser/browser.spec.ts +++ b/packages/zone.js/test/browser/browser.spec.ts @@ -1491,6 +1491,7 @@ describe('Zone', function() { logs.push(e.defaultPrevented.toString()); e.preventDefault(); logs.push(e.defaultPrevented.toString()); + expect(Zone.current.name).toBe(zone.name); }; zone.run(function() { @@ -1505,6 +1506,88 @@ 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()); + expect(Zone.current.name).toBe(zone.name); + }; + + const listener1 = (e: Event) => { + logs.push(e.defaultPrevented.toString()); + e.preventDefault(); + logs.push(e.defaultPrevented.toString()); + expect(Zone.current.name).toBe(zone.name); + }; + + 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()); + expect(Zone.current.name).toBe(zone.name); + }; + + const listener1 = (e: Event) => { + logs.push(e.defaultPrevented.toString()); + e.preventDefault(); + logs.push(e.defaultPrevented.toString()); + expect(Zone.current.name).toBe(zone.name); + }; + + 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) => {