Skip to content

Refactor/optimize event paths - #132

Open
tejaede wants to merge 4 commits into
mainfrom
refactor/optimize-event-paths
Open

Refactor/optimize event paths#132
tejaede wants to merge 4 commits into
mainfrom
refactor/optimize-event-paths

Conversation

@tejaede

@tejaede tejaede commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@tejaede
tejaede requested a review from marchant July 25, 2026 03:40
@tejaede
tejaede force-pushed the refactor/optimize-event-paths branch from bf97f4a to 02313ac Compare July 27, 2026 19:35
Comment thread core/event/event-manager.js Outdated
mutableEventTarget = mutableEvent.target;
eventPath = this.eventPathForTarget(mutableEventTarget);
mutableEvent._composedPath = eventPath;
// eventPath = this.eventPathForTarget(mutableEventTarget);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove now?

// this._previousEvent = event;
// }

// ... operation 1 ...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not very clear!

value: function (capture, target, eventType) {
let targetsByType = this._dispatchedTargetsByEventTypeByComposedPathMember.get(target),
dispatchedTargets = targetsByType && targetsByType.get(eventType),
dispatchedTargetsIterator = dispatchedTargets && dispatchedTargets.entries(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use the optional chaining operator ?. here:

            dispatchedTargetsIterator = dispatchedTargets?.entries(),


__clearCachedPathForTargetAndEventType: {
value: function (capture, target, eventType) {
if (capture) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bellow, if this._capturePathByTargetAndEventType has indeed target, we're accessing it 3 times:

this._capturePathByTargetAndEventType.has(target)
this._capturePathByTargetAndEventType.get(target)
this._capturePathByTargetAndEventType.get(target)

same in the else, would be great to optimize that if possible. The if could be:

if (this._capturePathByTargetAndEventType.get(target)?.has(eventType)) instead, right?

Or even with no if at all ?:

this._capturePathByTargetAndEventType.get(target)?.delete(eventType);


__invokeTargetListenersForEventPhase: {
value: function __invokeTargetListenersForEventPhase(iTarget, mutableEvent, phase, _eventType) {
value: function __invokeTargetListenersForEventPhase(iTarget, mutableEvent, phase, _eventType, phaseEventPath) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document why phaseEventPath is now an argument

tejaede and others added 3 commits August 17, 2026 21:55
Create a cache of the path for each event target
and type that only includes the targets in the
path that has a listener for the type
@marchant
marchant force-pushed the refactor/optimize-event-paths branch from 4195982 to 847f9b0 Compare August 18, 2026 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants