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
JiaLiPassion committed Mar 19, 2023
1 parent 8d99ad0 commit ebf8c88
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 47 deletions.
31 changes: 29 additions & 2 deletions packages/zone.js/lib/common/events.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ export function patchEventTarget(
if (task.isRemoved) {
return;
}
const options = task.options;
if (typeof options === 'object' && options?.passive === true) {
return;
}
const delegate = task.callback;
if (typeof delegate === 'object' && delegate.handleEvent) {
// create the bind version of handleEvent when invoke
Expand All @@ -126,7 +130,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 @@ -278,6 +281,14 @@ export function patchEventTarget(
}

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 (typeof taskData.options === 'object' && taskData.options?.passive === true) {
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 +421,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 = typeof options === 'object' ? options?.passive === true : false;

const zone = Zone.current;
let symbolEventNames = zoneSymbolEventNames[eventName];
Expand All @@ -434,6 +446,21 @@ export function patchEventTarget(
} else {
existingTasks = target[symbolEventName] = [];
}
if (passiveSupported && existingTasks.length) {
// check whether all tasks are passive.
let foundNonPassive = false;
for (let i = 0; i < existingTasks.length; i++) {
const o = existingTasks[i].options;
if (typeof o !== 'object' || o?.passive !== true) {
// same callback, same capture, same event name, just return
foundNonPassive = true;
break;
}
}
if (!foundNonPassive) {
isExisting = false;
}
}
let source;
const constructorName = target.constructor['name'];
const targetSource = globalSources[constructorName];
Expand All @@ -458,7 +485,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
168 changes: 123 additions & 45 deletions packages/zone.js/test/browser/browser.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1506,6 +1506,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 @@ -2968,54 +3046,54 @@ describe('Zone', function() {
}
});

it('should be able to continue to invoke remaining listeners even some listener throw error',
function(done: DoneFn) {
// override global.onerror to prevent jasmine report error
let oriWindowOnError = window.onerror;
window.onerror = function() {};
try {
let logs: string[] = [];
const listener1 = function() {
logs.push('listener1');
};
const listener2 = function() {
throw new Error('test1');
};
const listener3 = function() {
throw new Error('test2');
};
const listener4 = {
handleEvent: function() {
logs.push('listener2');
}
};

button.addEventListener('click', listener1);
button.addEventListener('click', listener2);
button.addEventListener('click', listener3);
button.addEventListener('click', listener4);
fit('should be able to continue to invoke remaining listeners even some listener throw error',
function(done: DoneFn) {
// override global.onerror to prevent jasmine report error
let oriWindowOnError = window.onerror;
window.onerror = function() {};
try {
let logs: string[] = [];
const listener1 = function() {
logs.push('listener1');
};
const listener2 = function() {
throw new Error('test1');
};
const listener3 = function() {
throw new Error('test2');
};
const listener4 = {
handleEvent: function() {
logs.push('listener2');
}
};

const mouseEvent = document.createEvent('MouseEvent');
mouseEvent.initEvent('click', true, true);
button.addEventListener('click', listener1);
button.addEventListener('click', listener2);
button.addEventListener('click', listener3);
button.addEventListener('click', listener4);

const unhandledRejection = (e: PromiseRejectionEvent) => {
logs.push(e.reason.message);
};
window.addEventListener('unhandledrejection', unhandledRejection);

button.dispatchEvent(mouseEvent);
expect(logs).toEqual(['listener1', 'listener2']);
const mouseEvent = document.createEvent('MouseEvent');
mouseEvent.initEvent('click', true, true);

setTimeout(() => {
expect(logs).toEqual(['listener1', 'listener2', 'test1', 'test2']);
window.removeEventListener('unhandledrejection', unhandledRejection);
window.onerror = oriWindowOnError;
done()
});
} catch (e: any) {
window.onerror = oriWindowOnError;
}
});
const unhandledRejection = (e: PromiseRejectionEvent) => {
logs.push(e.reason.message);
};
window.addEventListener('unhandledrejection', unhandledRejection);

button.dispatchEvent(mouseEvent);
expect(logs).toEqual(['listener1', 'listener2']);

setTimeout(() => {
expect(logs).toEqual(['listener1', 'listener2', 'test1', 'test2']);
window.removeEventListener('unhandledrejection', unhandledRejection);
window.onerror = oriWindowOnError;
done()
}, 100);
} catch (e: any) {
window.onerror = oriWindowOnError;
}
});

it('should be able to continue to invoke remaining listeners even some listener throw error with onHandleError zone',
function(done: DoneFn) {
Expand Down

0 comments on commit ebf8c88

Please sign in to comment.