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

Add listener API #3470

Closed
wants to merge 6 commits into from
Closed

Add listener API #3470

wants to merge 6 commits into from

Conversation

mikayla-maki
Copy link
Contributor

@mikayla-maki mikayla-maki commented Dec 1, 2023

This PR attempts to economize on the allocations made in 'library' crates (like ui2) by providing a wrapper around the most common pattern of callbacks used in the div element. Initially this was implemented with a IntoListener method that could take both a closure and a boxed closure. Unfortunately, due to rust-lang/rust#41078 (comment), it is impossible to have proper type inference for this pattern and the rust compiler's error is opaque. To fix this, we added a function with a normal closure parameter and made it a requirement at all callsites. I believe this tradeoff is an acceptable tradeoff for the composability and runtime performance gains that library-style crates receive.

Release Notes:

  • N/A

@mikayla-maki mikayla-maki changed the title Rework listeners (again) Allow listeners to be generic over boxed and unboxed closures. Dec 1, 2023
@mikayla-maki mikayla-maki changed the title Allow listeners to be generic over boxed and unboxed closures. Add listener API Dec 4, 2023
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.

1 participant