Skip to content

Commit d0e522f

Browse files
shirakabashirakaba
committed
fix: get listeners afresh each time
1 parent 6cb8a83 commit d0e522f

File tree

2 files changed

+15
-31
lines changed

2 files changed

+15
-31
lines changed

‎packages/core/data/dom-events/dom-event.ts

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ export class DOMEvent implements Event {
5454
Object.defineProperty(DOMEvent.prototype, 'currentTarget', { value: null, writable: true });
5555
Object.defineProperty(DOMEvent.prototype, 'target', { value: null, writable: true });
5656
Object.defineProperty(DOMEvent.prototype, 'propagationState', { value: EventPropagationState.resume, writable: true });
57-
Object.defineProperty(DOMEvent.prototype, 'listeners', { value: emptyArray, writable: true });
58-
Object.defineProperty(DOMEvent.prototype, 'listenersLazyCopy', { value: emptyArray, writable: true });
5957
}
6058

6159
declare NONE: 0;
@@ -265,7 +263,7 @@ export class DOMEvent implements Event {
265263
*/
266264
// Taking multiple params rather than a single property bag saves about 100
267265
// nanoseconds per call.
268-
dispatchTo(target: Observable, data: EventData, getGlobalEventHandlersPreHandling?: () => ListenerEntry[], getGlobalEventHandlersPostHandling?: () => ListenerEntry[]): boolean {
266+
dispatchTo(target: Observable, data: EventData, getGlobalEventHandlers?: (data: EventData, eventType: 'First' | '') => readonly ListenerEntry[] | undefined): boolean {
269267
if (this.eventPhase !== DOMEvent.NONE) {
270268
throw new Error('Tried to dispatch a dispatching event');
271269
}
@@ -306,8 +304,7 @@ export class DOMEvent implements Event {
306304
// event. This keeps behaviour as consistent with DOM Events as
307305
// possible.
308306

309-
this.listeners = getGlobalEventHandlersPreHandling?.() || emptyArray;
310-
this.handleEvent(data, true, DOMEvent.CAPTURING_PHASE, removeGlobalEventListener, target.constructor);
307+
this.handleEvent(data, true, () => getGlobalEventHandlers?.(data, 'First') || emptyArray, DOMEvent.CAPTURING_PHASE, removeGlobalEventListener, target.constructor);
311308

312309
const eventPath = this.getEventPath(target, 'capture');
313310

@@ -319,8 +316,7 @@ export class DOMEvent implements Event {
319316
this.currentTarget = currentTarget;
320317
this.eventPhase = this.target === this.currentTarget ? DOMEvent.AT_TARGET : DOMEvent.CAPTURING_PHASE;
321318

322-
this.listeners = currentTarget.getEventList(this.type) || emptyArray;
323-
this.handleEvent(data, false, DOMEvent.CAPTURING_PHASE, currentTarget.removeEventListener, currentTarget);
319+
this.handleEvent(data, false, () => currentTarget.getEventList(this.type) || emptyArray, DOMEvent.CAPTURING_PHASE, currentTarget.removeEventListener, currentTarget);
324320

325321
if (this.propagationState !== EventPropagationState.resume) {
326322
this.resetForRedispatch();
@@ -335,8 +331,7 @@ export class DOMEvent implements Event {
335331
this.currentTarget = currentTarget;
336332
this.eventPhase = this.target === this.currentTarget ? DOMEvent.AT_TARGET : DOMEvent.BUBBLING_PHASE;
337333

338-
this.listeners = currentTarget.getEventList(this.type) || emptyArray;
339-
this.handleEvent(data, false, DOMEvent.BUBBLING_PHASE, currentTarget.removeEventListener, currentTarget);
334+
this.handleEvent(data, false, () => currentTarget.getEventList(this.type) || emptyArray, DOMEvent.BUBBLING_PHASE, currentTarget.removeEventListener, currentTarget);
340335

341336
if (this.propagationState !== EventPropagationState.resume) {
342337
this.resetForRedispatch();
@@ -356,24 +351,23 @@ export class DOMEvent implements Event {
356351
this.eventPhase = DOMEvent.BUBBLING_PHASE;
357352
}
358353

359-
this.listeners = getGlobalEventHandlersPostHandling?.() || emptyArray;
360-
this.handleEvent(data, true, DOMEvent.BUBBLING_PHASE, removeGlobalEventListener, target.constructor);
354+
this.handleEvent(data, true, () => getGlobalEventHandlers?.(data, '') || emptyArray, DOMEvent.BUBBLING_PHASE, removeGlobalEventListener, target.constructor);
361355

362356
this.resetForRedispatch();
363357
return !this.defaultPrevented;
364358
}
365359

366360
// Taking multiple params instead of a single property bag saves 250
367361
// nanoseconds per dispatchTo() call.
368-
private handleEvent(data: EventData, isGlobal: boolean, phase: 0 | 1 | 2 | 3, removeEventListener: (eventName: string, callback?: any, thisArg?: any, capture?: boolean) => void, removeEventListenerContext: unknown) {
362+
private handleEvent(data: EventData, isGlobal: boolean, getListeners: () => readonly ListenerEntry[], phase: 0 | 1 | 2 | 3, removeEventListener: (eventName: string, callback?: any, thisArg?: any, capture?: boolean) => void, removeEventListenerContext: unknown) {
369363
// Clone the array just before any mutations. I tried swapping this out
370364
// for a copy-on-write array, but as it had to maintain its own array of
371365
// listeners for any write actions, it actually ran significantly
372366
// slower.
373367
//
374368
// There's no clear observable difference between array spread and slice
375369
// here, but I think slice has reason to run faster.
376-
const listeners = this.listeners.slice();
370+
const listeners = getListeners().slice();
377371

378372
for (let i = listeners.length - 1; i >= 0; i--) {
379373
const listener = listeners[i];
@@ -402,7 +396,7 @@ export class DOMEvent implements Event {
402396
// MutationSensitiveArray called afterRemoval, similar to
403397
// beforeMutation) to allow O(1) lookup, but it went 1000 ns slower
404398
// in practice, so it stays!
405-
if (!this.listeners.includes(listener)) {
399+
if (!getListeners().includes(listener)) {
406400
continue;
407401
}
408402

@@ -456,6 +450,5 @@ export class DOMEvent implements Event {
456450
this.target = null;
457451
this.eventPhase = DOMEvent.NONE;
458452
this.propagationState = EventPropagationState.resume;
459-
this.listeners = emptyArray;
460453
}
461454
}

‎packages/core/data/observable/index.ts

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,7 @@ export class Observable implements EventTarget {
376376
* @param options Options for the event, in line with DOM Standard.
377377
*/
378378
public notify<T extends EventData>(data: T, options?: CustomEventInit): void {
379-
new DOMEvent(data.eventName, options).dispatchTo(
380-
this,
381-
data,
382-
() => this._getGlobalEventHandlers(data, 'First'),
383-
() => this._getGlobalEventHandlers(data, '')
384-
);
379+
new DOMEvent(data.eventName, options).dispatchTo(this, data, Observable._getGlobalEventHandlers);
385380
}
386381

387382
dispatchEvent(event: DOMEvent): boolean {
@@ -391,19 +386,15 @@ export class Observable implements EventTarget {
391386
detail: event.detail,
392387
};
393388

394-
return event.dispatchTo(
395-
this,
396-
data,
397-
() => this._getGlobalEventHandlers(data, 'First'),
398-
() => this._getGlobalEventHandlers(data, '')
399-
);
389+
return event.dispatchTo(this, data, Observable._getGlobalEventHandlers);
400390
}
401391

402-
private _getGlobalEventHandlers(data: EventData, eventType: 'First' | ''): ListenerEntry[] {
392+
private static _getGlobalEventHandlers(data: EventData, eventType: 'First' | ''): readonly ListenerEntry[] | undefined {
403393
const eventClass = data.object?.constructor?.name;
404-
const globalEventHandlersForOwnClass = _globalEventHandlers[eventClass]?.[`${data.eventName}${eventType}`] ?? [];
405-
const globalEventHandlersForAllClasses = _globalEventHandlers['*']?.[`${data.eventName}${eventType}`] ?? [];
406-
return [...globalEventHandlersForOwnClass, ...globalEventHandlersForAllClasses];
394+
const globalEventHandlersForOwnClass = _globalEventHandlers[eventClass]?.[`${data.eventName}${eventType}`];
395+
const globalEventHandlersForAllClasses = _globalEventHandlers['*']?.[`${data.eventName}${eventType}`];
396+
397+
return globalEventHandlersForOwnClass?.length ? (globalEventHandlersForAllClasses?.length ? [...globalEventHandlersForOwnClass, ...globalEventHandlersForAllClasses] : globalEventHandlersForOwnClass) : globalEventHandlersForAllClasses?.length ? globalEventHandlersForAllClasses : undefined;
407398
}
408399

409400
/**

0 commit comments

Comments
 (0)