Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent event arguments and documentation #69

Merged
merged 2 commits into from
Mar 3, 2017

Conversation

clue
Copy link
Member

@clue clue commented Mar 3, 2017

This simple PR changes it so that we only emit event arguments that are actually part of the event, i.e. we no longer emit the instance this event was emitted on. This was actually an undocumented feature, so some component may have relied on this.

Among others, this fixes several issues because these instance where never emitted consistently in the first place. In particular, the forwardEvents helper is used throughout React's ecosystem and has never actually updated the instances, as it has never checked any of the event arguments.

I've updated the documentation to now explicitly state which arguments are actually passed to the event handlers.

Not passing these unneeded instances also helps with improving performance slightly. On my system, the benchmark examples went from ~1980 MiB/s to ~2020 MiB/s.

In case the instance the event was emitted on is needed, use a closure and pass it in manually.

Note that this is potentially a BC break in case anybody relied on the undocumented event arguments.

@clue clue added this to the v0.5.0 milestone Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants