-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Nightly features #2743
Nightly features #2743
Conversation
Visit the preview URL for this PR (updated for commit 0b3c967): https://yew-rs-api--pr2743-nightly-features-vychbm58.web.app (expires Fri, 01 Jul 2022 14:12:47 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Size Comparison
✅ None of the examples has changed their size significantly. |
mod feat_nightly { | ||
use super::*; | ||
|
||
impl FnOnce<()> for UseForceUpdate { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not type UseForceUpdate = Rc<dyn Fn()>
?
If we do not wish users to assume the underlying type of the handle, we can use type UseForceUpdate = impl Fn();
for nightly.
Also maybe should be named UseForceUpdateHandle
so it's consistent with other hooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not
type UseForceUpdate = Rc<dyn Fn()>
?
@WorldSEnder can better answer that. They added it in #2586
If we do not wish users to assume the underlying type of the handle, we can use
type UseForceUpdate = impl Fn();
for nightly.
There's no point of doing that since the underlying type is Rc<dyn Fn()>
. We can't use impl Fn()
there because it is created as:
yew/packages/yew/src/functional/mod.rs
Line 356 in 015412e
Rc::new(move || link.send_message(())) |
Replacing that with impl Fn
fails because the type alias can't capture the type parameter T
.
Also maybe should be named
UseForceUpdateHandle
so it's consistent with other hooks.
Why not just expose ReRender
type instead of having
type UseForceUpdateHandle = ReRender;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason was to not expose the fact that it's an Rc<_>
since that's an implementation detail and the exact smart pointer in use might change. I'm not beefed up on the ABI of impl Trait
for this case, but I think it makes it impossible to add other methods (and at least Sync
/Send
would have to be added anyway). That part is easier to control with a struct.
Also maybe should be named UseForceUpdateHandle
No objections. At the time I thought about Handle
more for stateful things and just named it after the hook, but some consistency would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with having the option to add more methods on the type
This reverts commit 761dbb0.
Description
Add a
nightly
feature to use unstable features. This PR adds the following features:UseForceUpdate
hasFn
/FnMut
/FnOnce
implementations callingforce_trigger
UseReducerHandle
hasFn
/FnMut
/FnOnce
implementations callingdispatch
use_prepared_state!
does not rewrite async closures to closure returning async blockFixes #2681
Checklist