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 
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 angular#45020
  • Loading branch information
@JiaLiPassion
JiaLiPassion committed Dec 18, 2023
1 parent 8bf7525 commit 6860a6e
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 25 deletions.
66 changes: 41 additions & 25 deletions packages/zone.js/lib/common/events.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 :
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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) {
Expand Down
83 changes: 83 additions & 0 deletions packages/zone.js/test/browser/browser.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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) => {
Expand Down

0 comments on commit 6860a6e

Please sign in to comment.