Skip to content

Discussion: refactor use eventhandler to dispatch events#51

Closed
keithamus wants to merge 6 commits intofix-listenforbind-allow-any-node-to-be-given-as-first-paramfrom
refactor-use-eventhandler-to-dispatch-events
Closed

Discussion: refactor use eventhandler to dispatch events#51
keithamus wants to merge 6 commits intofix-listenforbind-allow-any-node-to-be-given-as-first-paramfrom
refactor-use-eventhandler-to-dispatch-events

Conversation

@keithamus
Copy link
Copy Markdown
Contributor

When discussing performance concerns with listenForBind() (a discussion motivated by #50), we looked at ways we could optimise the performance of the addEventListener code.

One of the slowest parts of this was creating an anonymous function in a loop. A way around this is to make a centralized event handler which does the heavy lifting. We add some cheap checks inside bindActionsToController - just enough to find out what to bind, then in handleEvent we can do the more detailed checks. This overall looks to work out a bit better (based on my rudimentary benchmarking) for the initial bind than the closure.

The nice thing about this refactoring is it also paves the way for us to listen for mutations on the data-action attribute; we can simply call bindActionsToController if the attribute changes, and it just... works itself out - it avoids double bindings, and unbinding happens automatically because the string check occurs at event dispatch instead of ahead of it.

@keithamus keithamus requested a review from a team as a code owner July 1, 2020 17:41
Comment thread src/bind.ts Outdated
@keithamus
Copy link
Copy Markdown
Contributor Author

Superseded by #53

@keithamus keithamus closed this Jul 20, 2020
@keithamus keithamus deleted the refactor-use-eventhandler-to-dispatch-events branch July 21, 2020 14:26
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