diff --git a/packages/zone.js/lib/common/events.ts b/packages/zone.js/lib/common/events.ts index b7ea63dd8cc..2b752f0731d 100644 --- a/packages/zone.js/lib/common/events.ts +++ b/packages/zone.js/lib/common/events.ts @@ -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; @@ -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; } @@ -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; - } - return newOptions; + if (typeof options !== 'object' || options === null) { + return options; } - - 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; } const makeAddListener = function ( @@ -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. diff --git a/packages/zone.js/test/browser/browser.spec.ts b/packages/zone.js/test/browser/browser.spec.ts index 218536716ea..c9bdd412d16 100644 --- a/packages/zone.js/test/browser/browser.spec.ts +++ b/packages/zone.js/test/browser/browser.spec.ts @@ -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[] = [];