Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 22 additions & 21 deletions packages/zone.js/lib/common/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ export const globalSources: any = {};
const EVENT_NAME_SYMBOL_REGX = new RegExp('^' + ZONE_SYMBOL_PREFIX + '(\\w+)(true|false)$');
const IMMEDIATE_PROPAGATION_SYMBOL = zoneSymbol('propagationStopped');

// Recognised members of the `AddEventListenerOptions` WebIDL dictionary, used
// by `copyEventListenerOptions` to recover accessors and non-enumerable
// properties that a caller-supplied options object may expose.
const KNOWN_EVENT_LISTENER_OPTIONS = ['capture', 'once', 'passive', 'signal'];

function prepareEventNames(eventName: string, eventNameToString?: (eventName: string) => string) {
const falseEventName = (eventNameToString ? eventNameToString(eventName) : eventName) + FALSE_STR;
const trueEventName = (eventNameToString ? eventNameToString(eventName) : eventName) + TRUE_STR;
Expand Down Expand Up @@ -381,7 +386,8 @@ export function patchEventTarget(
return {passive: true};
}
if (typeof options === 'object' && options.passive !== false) {
return {...options, passive: true};
options.passive = true;
return options;
}
return options;
}
Expand Down Expand Up @@ -492,27 +498,22 @@ export function patchEventTarget(
const passiveEvents: string[] = _global[zoneSymbol('PASSIVE_EVENTS')];

function copyEventListenerOptions(options: any) {
if (typeof options === 'object' && options !== null) {
// We need to destructure the target `options` object since it may
// be frozen or sealed (possibly provided implicitly by a third-party
// library), or its properties may be readonly.
const newOptions: any = {...options};
// The `signal` option was recently introduced, which caused regressions in
// third-party scenarios where `AbortController` was directly provided to
// `addEventListener` as options. For instance, in cases like
// `document.addEventListener('keydown', callback, abortControllerInstance)`,
// which is valid because `AbortController` includes a `signal` getter, spreading
// `{...options}` wouldn't copy the `signal`. Additionally, using `Object.create`
// isn't feasible since `AbortController` is a built-in object type, and attempting
// to create a new object directly with it as the prototype might result in
// unexpected behavior.
if (options.signal) {
newOptions.signal = options.signal;
if (typeof options !== 'object' || options === null) {
return options;
}
// Spread copies own enumerable properties, invoking any getters exactly once.
const newOptions: any = {...options};
// Anything the spread could not see (inherited accessors such as
// `AbortController.prototype.signal`, or non-enumerable properties defined
// via `Object.defineProperty`) is read directly from the source, exactly once. Reading
// from `options` rather than `newOptions` also gives prototype getters the
// correct receiver.
for (const key of KNOWN_EVENT_LISTENER_OPTIONS) {
if (!Object.hasOwn(newOptions, key) && key in options) {
newOptions[key] = options[key];
}
return newOptions;
}

return options;
return newOptions;
}

const makeAddListener = function (
Expand Down Expand Up @@ -556,7 +557,7 @@ export function patchEventTarget(
}

const passive = !!passiveEvents && passiveEvents.indexOf(eventName) !== -1;
const options = copyEventListenerOptions(buildEventListenerOptions(arguments[2], passive));
const options = buildEventListenerOptions(copyEventListenerOptions(arguments[2]), passive);
const signal: AbortSignal | undefined = options?.signal;
if (signal?.aborted) {
// the signal is an aborted one, just return without attaching the event listener.
Expand Down
67 changes: 67 additions & 0 deletions packages/zone.js/test/browser/browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2184,6 +2184,73 @@ describe('Zone Browser', function () {
expect(logs).toEqual(['click2']);
});

// Options exposed as accessors or non-enumerable properties must still
// reach the native call. https://github.com/angular/angular/issues/54142
describe('event listener options built with accessors', function () {
it('should invoke a non-enumerable `passive` getter (MDN feature-detect pattern)', function () {
let getCount = 0;
const opts = Object.defineProperty({}, 'passive', {
get: () => {
getCount++;
return false;
},
});
const listener = () => {};
button.addEventListener('click', listener, opts as any);
expect(getCount).toBe(1);
button.removeEventListener('click', listener, opts as any);
});

it('should honour `capture: true` supplied as an accessor', function () {
const opts = Object.defineProperty({}, 'capture', {get: () => true});
const inner = document.createElement('span');
button.appendChild(inner);
let phase = -1;
const listener = (e: Event) => {
phase = e.eventPhase;
};
button.addEventListener('click', listener, opts as any);
inner.dispatchEvent(clickEvent);
expect(phase).toBe(Event.CAPTURING_PHASE);

button.removeEventListener('click', listener, opts as any);
phase = -1;
inner.dispatchEvent(clickEvent);
expect(phase).toBe(-1);
button.removeChild(inner);
});

it('should honour `once: true` supplied as an accessor', function () {
const opts = Object.defineProperty({}, 'once', {get: () => true});
let callCount = 0;
button.addEventListener('click', () => callCount++, opts as any);
button.dispatchEvent(clickEvent);
button.dispatchEvent(clickEvent);
expect(callCount).toBe(1);
});

// `AbortController.prototype.signal` is a prototype accessor, so an
// own-properties-only copy would drop it.
it('should honour `signal` on an AbortController passed as options', function () {
const ac = new AbortController();
const logs: string[] = [];
button.addEventListener('click', () => logs.push('click'), ac);
button.dispatchEvent(clickEvent);
ac.abort();
button.dispatchEvent(clickEvent);
expect(logs).toEqual(['click']);
expect(button.eventListeners!('click').length).toBe(0);
});

// https://github.com/angular/angular/pull/55796
it('should accept a frozen options object', function () {
const opts = Object.freeze({capture: true, once: true});
const listener = () => {};
expect(() => button.addEventListener('click', listener, opts as any)).not.toThrow();
button.removeEventListener('click', listener, opts as any);
});
});

// https://github.com/angular/angular/issues/56148
it('should store the remove abort listener on the task itself and not the task data', function () {
const logs: string[] = [];
Expand Down
Loading