Skip to content

Commit 7d39d6e

Browse files
asynclizcopybara-github
authored andcommitted
refactor: no longer re-dispatches an event copy for afterDispatch() logic
PiperOrigin-RevId: 900881800
1 parent 92883af commit 7d39d6e

2 files changed

Lines changed: 260 additions & 59 deletions

File tree

internal/events/dispatch-hooks.ts

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export function afterDispatch(event: Event, callback: () => void) {
7777
throw new Error(`'${event.type}' event needs setupDispatchHooks().`);
7878
}
7979

80-
hooks.addEventListener('after', callback);
80+
hooks.addEventListener('after', callback, {once: true});
8181
}
8282

8383
/**
@@ -122,55 +122,58 @@ export function setupDispatchHooks(
122122

123123
for (const eventType of eventTypes) {
124124
// Don't register multiple dispatch hook listeners. A second registration
125-
// would lead to the second listener re-dispatching a re-dispatched event,
126-
// which can cause an infinite loop inside the other one.
125+
// would lead to the second listener calling `afterDispatch()` hooks twice.
127126
if (typesAlreadySetUp.has(eventType)) {
128127
continue;
129128
}
130129

131-
// When we re-dispatch the event, it's going to immediately trigger this
132-
// listener again. Use a flag to ignore it.
133-
let isRedispatching = false;
134130
element.addEventListener(
135131
eventType,
136132
(event: Event) => {
137-
if (isRedispatching) {
138-
return;
139-
}
140-
141-
// Do not let the event propagate to any other listener (not just
142-
// bubbling listeners with `stopPropagation()`).
143-
event.stopImmediatePropagation();
144-
// Make a copy.
145-
const eventCopy = Reflect.construct(event.constructor, [
146-
event.type,
147-
event,
148-
]);
149-
150133
// Add hooks onto the event.
151134
const hooks = new EventTarget();
152-
(eventCopy as EventWithDispatchHooks)[dispatchHooks] = hooks;
135+
(event as EventWithDispatchHooks)[dispatchHooks] = hooks;
153136

154-
// Re-dispatch the event. We can't reuse `redispatchEvent()` since we
155-
// need to add the hooks to the copy before it's dispatched.
156-
isRedispatching = true;
157-
const composedPathIncludesAnchor = event
158-
.composedPath()
159-
.some((el) => (el as Partial<HTMLElement>)?.matches?.('a'));
160-
if (event.type === 'click' && composedPathIncludesAnchor) {
161-
// For legacy reasons, synthetic click events dispatching on
162-
// HTMLAnchorElement will trigger link behavior. Prevent this since
163-
// we will dispatch a copy of the same click event.
164-
event.preventDefault();
165-
}
166-
const dispatched = event.composedPath()[0].dispatchEvent(eventCopy);
167-
isRedispatching = false;
168-
if (!dispatched) {
169-
event.preventDefault();
137+
const cleanupLastNodeListener = new AbortController();
138+
const callAfterDispatch = () => {
139+
cleanupLastNodeListener.abort();
140+
hooks.dispatchEvent(new Event('after'));
141+
};
142+
143+
const patchStopPropagation = (
144+
superMethod: Event['stopPropagation'],
145+
) => {
146+
return function (this: Event) {
147+
superMethod.call(this);
148+
// Synchronously call afterDispatch() hooks when interrupted.
149+
callAfterDispatch();
150+
};
151+
};
152+
153+
event.stopPropagation = patchStopPropagation(event.stopPropagation);
154+
event.stopImmediatePropagation = patchStopPropagation(
155+
event.stopImmediatePropagation,
156+
);
157+
158+
// Add an event listener to detect the end of the event's propagation.
159+
const composedPath = event.composedPath();
160+
let lastNodeForEvent: EventTarget;
161+
if (event.composed && event.bubbles) {
162+
lastNodeForEvent = composedPath[composedPath.length - 1];
163+
} else if (!event.bubbles) {
164+
lastNodeForEvent = composedPath[0];
165+
} else {
166+
lastNodeForEvent = (composedPath[0] as Element).getRootNode();
170167
}
171168

172-
// Synchronously call afterDispatch() hooks.
173-
hooks.dispatchEvent(new Event('after'));
169+
lastNodeForEvent.addEventListener(
170+
eventType,
171+
() => {
172+
// Synchronously call afterDispatch() hooks.
173+
callAfterDispatch();
174+
},
175+
{once: true, signal: cleanupLastNodeListener.signal},
176+
);
174177
},
175178
{
176179
// Ensure this listener runs before other listeners.

internal/events/dispatch-hooks_test.ts

Lines changed: 219 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -54,27 +54,6 @@ describe('dispatch hooks', () => {
5454
.withContext('innerClickListener')
5555
.toHaveBeenCalledTimes(1);
5656
});
57-
58-
it('should not trigger activation behavior for clicks coming from inner <a> elements', () => {
59-
const shadowRoot = element.attachShadow({mode: 'open'});
60-
const anchorElement = document.createElement('a');
61-
anchorElement.href = '#';
62-
shadowRoot.appendChild(anchorElement);
63-
64-
setupDispatchHooks(element, 'click');
65-
66-
const clickEvent = new MouseEvent('click', {
67-
bubbles: true,
68-
cancelable: true,
69-
composed: true,
70-
});
71-
72-
anchorElement.dispatchEvent(clickEvent);
73-
74-
expect(clickEvent.defaultPrevented)
75-
.withContext('clickEvent.defaultPrevented')
76-
.toBeTrue();
77-
});
7857
});
7958

8059
describe('afterDispatch()', () => {
@@ -212,5 +191,224 @@ describe('dispatch hooks', () => {
212191
.withContext('afterDispatch() callback')
213192
.toHaveBeenCalledTimes(1);
214193
});
194+
195+
it('is called after parent event listeners are called', () => {
196+
setupDispatchHooks(element, 'click');
197+
198+
const callOrder: string[] = [];
199+
element.addEventListener('click', (event) => {
200+
callOrder.push('element@click');
201+
afterDispatch(event, () => {
202+
callOrder.push('afterDispatch');
203+
});
204+
});
205+
document.addEventListener('click', () => {
206+
callOrder.push('parent@click');
207+
});
208+
209+
element.click();
210+
211+
const expectedCallOrder = [
212+
'element@click',
213+
'parent@click',
214+
'afterDispatch',
215+
];
216+
expect(callOrder)
217+
.withContext('call order of event listeners and afterDispatch()')
218+
.toEqual(expectedCallOrder);
219+
});
220+
221+
it('is called after other event listeners for non-bubbling events', () => {
222+
setupDispatchHooks(element, 'change');
223+
224+
const callOrder: string[] = [];
225+
element.addEventListener('change', (event) => {
226+
callOrder.push('element@change');
227+
afterDispatch(event, () => {
228+
callOrder.push('afterDispatch');
229+
});
230+
});
231+
element.addEventListener('change', () => {
232+
callOrder.push('element@change2');
233+
});
234+
235+
element.dispatchEvent(new Event('change'));
236+
237+
const expectedCallOrder = [
238+
'element@change',
239+
'element@change2',
240+
'afterDispatch',
241+
];
242+
expect(callOrder)
243+
.withContext('call order of event listeners and afterDispatch()')
244+
.toEqual(expectedCallOrder);
245+
});
246+
247+
it('is called after other event listeners for bubbling non-composed events in a shadow root', () => {
248+
const shadowRoot = element.attachShadow({mode: 'open'});
249+
const child = document.createElement('div');
250+
shadowRoot.appendChild(child);
251+
252+
setupDispatchHooks(child, 'custom-event');
253+
254+
const callOrder: string[] = [];
255+
child.addEventListener('custom-event', (event) => {
256+
callOrder.push('child@custom-event');
257+
afterDispatch(event, () => {
258+
callOrder.push('afterDispatch');
259+
});
260+
});
261+
shadowRoot.addEventListener('custom-event', () => {
262+
callOrder.push('shadowRoot@custom-event');
263+
});
264+
const elementListener = jasmine.createSpy('elementListener');
265+
element.addEventListener('custom-event', elementListener);
266+
267+
child.dispatchEvent(
268+
new Event('custom-event', {bubbles: true, composed: false}),
269+
);
270+
271+
const expectedCallOrder = [
272+
'child@custom-event',
273+
'shadowRoot@custom-event',
274+
'afterDispatch',
275+
];
276+
277+
expect(callOrder)
278+
.withContext('call order of event listeners and afterDispatch()')
279+
.toEqual(expectedCallOrder);
280+
expect(elementListener)
281+
.withContext(
282+
'listener on element with shadow root should not be called for `composed: false` event',
283+
)
284+
.not.toHaveBeenCalled();
285+
});
286+
287+
it('is called when parent non-root event listeners stop propagation', () => {
288+
setupDispatchHooks(element, 'click');
289+
290+
/*
291+
#document (root - should not be called)
292+
element (parent - stops propagation)
293+
child (child - calls afterDispatch)
294+
*/
295+
const child = document.createElement('div');
296+
element.appendChild(child);
297+
const childAfterDispatchCallback = jasmine.createSpy(
298+
'childAfterDispatchCallback',
299+
);
300+
child.addEventListener('click', (event) => {
301+
afterDispatch(event, childAfterDispatchCallback);
302+
});
303+
element.addEventListener('click', (event) => {
304+
event.stopPropagation();
305+
});
306+
const rootClickListener = jasmine.createSpy('rootClickListener');
307+
document.addEventListener('click', rootClickListener);
308+
309+
child.click();
310+
311+
expect(rootClickListener)
312+
.withContext('root click listener')
313+
.not.toHaveBeenCalled();
314+
expect(childAfterDispatchCallback)
315+
.withContext('child afterDispatch() callback')
316+
.toHaveBeenCalledTimes(1);
317+
});
318+
319+
it('is called when parent non-root event listeners immediately stops propagation', () => {
320+
setupDispatchHooks(element, 'click');
321+
322+
/*
323+
element (parent - stops propagation immediately)
324+
child (child - calls afterDispatch)
325+
*/
326+
const child = document.createElement('div');
327+
element.appendChild(child);
328+
const childAfterDispatchCallback = jasmine.createSpy(
329+
'childAfterDispatchCallback',
330+
);
331+
child.addEventListener('click', (event) => {
332+
afterDispatch(event, childAfterDispatchCallback);
333+
});
334+
element.addEventListener('click', (event) => {
335+
event.stopImmediatePropagation();
336+
});
337+
const additionalClickListener = jasmine.createSpy(
338+
'notCalledClickListener',
339+
);
340+
document.addEventListener('click', additionalClickListener);
341+
342+
child.click();
343+
344+
expect(additionalClickListener)
345+
.withContext('additional click listener after propagation is stopped')
346+
.not.toHaveBeenCalled();
347+
expect(childAfterDispatchCallback)
348+
.withContext('child afterDispatch() callback')
349+
.toHaveBeenCalledTimes(1);
350+
});
351+
352+
it('is DOES NOT support being called after the execution of the event listener that stopped propagation', () => {
353+
setupDispatchHooks(element, 'click');
354+
355+
const child = document.createElement('div');
356+
element.appendChild(child);
357+
const callOrder: string[] = [];
358+
child.addEventListener('click', (event) => {
359+
callOrder.push('child@click');
360+
afterDispatch(event, () => {
361+
callOrder.push('afterDispatch');
362+
});
363+
});
364+
element.addEventListener('click', (event) => {
365+
callOrder.push('parent@click');
366+
event.stopPropagation();
367+
callOrder.push('parent done');
368+
});
369+
370+
child.click();
371+
372+
// Ideally, when the event stops propagating, afterDispatch() is called
373+
// directly after the execution of the function that stops propagation.
374+
// However, this would mean introducing asynchronicity, which is not
375+
// allowed. The compromise is to synchronously call afterDispatch() hooks
376+
// when propagation is stopped.
377+
const desiredCallOrder = [
378+
'child@click',
379+
'parent@click',
380+
'parent done',
381+
'afterDispatch',
382+
];
383+
const expectedCallOrder = [
384+
'child@click',
385+
'parent@click',
386+
'afterDispatch',
387+
'parent done',
388+
];
389+
expect(callOrder)
390+
.withContext('call order of event listeners and afterDispatch()')
391+
.toEqual(expectedCallOrder);
392+
expect(callOrder).not.toEqual(desiredCallOrder);
393+
});
394+
395+
it('is not called multiple times if stopPropagation() is called multiple times', () => {
396+
setupDispatchHooks(element, 'click');
397+
398+
const afterDispatchCallback = jasmine.createSpy('afterDispatchCallback');
399+
element.addEventListener('click', (event) => {
400+
afterDispatch(event, afterDispatchCallback);
401+
});
402+
element.addEventListener('click', (event) => {
403+
event.stopPropagation();
404+
event.stopPropagation();
405+
});
406+
407+
element.click();
408+
409+
expect(afterDispatchCallback)
410+
.withContext('afterDispatch() callback')
411+
.toHaveBeenCalledTimes(1);
412+
});
215413
});
216414
});

0 commit comments

Comments
 (0)