Skip to content

Commit

Permalink
fix(zone.js): should allow add passive/non-passive listeners together
Browse files Browse the repository at this point in the history
Close angular#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.
  • Loading branch information
JiaLiPassion committed Dec 14, 2023
1 parent 629343f commit 59d44f0
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 4 deletions.
32 changes: 29 additions & 3 deletions packages/zone.js/lib/common/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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];
Expand All @@ -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];
Expand All @@ -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) {
Expand Down
80 changes: 79 additions & 1 deletion packages/zone.js/test/browser/browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 59d44f0

Please sign in to comment.