feat: add clonable immutable Handler#146
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
=======================================
Coverage 89.74% 89.75%
=======================================
Files 31 31
Lines 5139 5171 +32
Branches 5139 5171 +32
=======================================
+ Hits 4612 4641 +29
- Misses 427 430 +3
Partials 100 100
🚀 New features to boost your workflow:
|
|
Thanks for the PR! I like the idea, but I do think it might be a bit confusing. You changed I wonder if having two functions, e.g. |
Currently yeah, I think so. As far as I understand, there are two key differences here: 1.
|
|
Okay, I'm convinced that this change is the right thing to do. Let me just bikeshed over the name for a bit. To me, I think it would be slightly more intuitive if the types were... pub struct Handler<T>(bool, Arc<dyn Fn(T) + Send + Sync + 'static>);
pub struct HandlerMut<'a, T>(bool, Box<dyn FnMut(T) + Send + Sync + 'a>);This makes it more of a breaking change, but I think it's justified. |
|
Ok, that's more in line with rust stdlib naming ( |
There was a problem hiding this comment.
Sorry for being so slow to respond on this one. I'm just trying to give myself time to think of any other hidden gotchas here. But I think it looks good. Just one more comment on some ways the docs could possibly be improved. Let me know what you think.
Thanks so much for this!
packages/iocraft/src/handler.rs
Outdated
| } | ||
| } | ||
|
|
||
| /// Immutable variant of [`HandlerMut`]: it lacks ability to mutate captured variables, but can be cloned. |
There was a problem hiding this comment.
I think it's important to point out that this can be turned into HandlerMut. Maybe say something like...
Just as
Fncan be used where anFnMutis expected,Handlercan be used where aHandlerMutis expected viaFrom/Into.
Also, it is probably worth pointing out that components should usually accept properties via HandlerMut instead of Handler.
There was a problem hiding this comment.
Tried my best at writing docs for Handler, and also included an example. It still is a bit confusing, but at least there is a clear and communicated rationale for the distinction.
|
Thanks again for this! 🚀 |
What It Does
Add an immutable variant of
Handler.By default,
Handlercan mutate captured values. It's useful for buttons and other components that can mutate state directly, but it limits reusability of theHandler, especially when used withhooks.use_async_handler.For example, I have a handler that reacts to component actions:
It is then impossible to use this action twice for two different buttons because it can't be copied, e.g.
Making a special
RefHandlerthat can be cloned resolve this issue (which is even easier with the newRefHandler::bindfunction). And implementingimpl From<RefHandler> for Handlerintroduces no breaking changes.RefHandler might be a wrong name ? I was considering AsyncHandler or ConstHandler, but RefHandler seemed more appropriate with the rest of the lib's naming ? I'm open to changing it.
Related Issues
None.