-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Implement automatic conversion for primitives #3533
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
base: master
Are you sure you want to change the base?
Conversation
Visit the preview URL for this PR (updated for commit 0cc3926): https://yew-rs-api--pr3533-automatically-cast-p-8kwidweh.web.app (expires Mon, 27 Nov 2023 08:31:36 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Benchmark - coreYew Master
Pull Request
|
@@ -251,7 +251,7 @@ pub use yew_macro::html_nested; | |||
/// # assert_eq!(props.name, "Minka"); | |||
/// // ... or build the associated properties of a component | |||
/// let props = yew::props!(MyComponent::Properties { | |||
/// id: 2, | |||
/// id: 2_usize, |
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.
This issue is REALLY unfortunate. The compiler used to guess that type properly before but with this change it seems it's impossible to make it work again. I don't know why. But I think the pros outweigh the cons.
error[E0277]: the trait bound `i32: IntoPropValue<usize>` is not satisfied
--> packages/yew/src/lib.rs:255:9
|
32 | id: 2,
| -- ^ the trait `IntoPropValue<usize>` is not implemented for `i32`
| |
| required by a bound introduced by this call
|
= help: the following other types implement trait `IntoPropValue<T>`:
<i32 as IntoPropValue<i64>>
<i32 as IntoPropValue<i128>>
<i32 as IntoPropValue<f64>>
<i32 as IntoPropValue<VNode>>
<i32 as IntoPropValue<Option<i64>>>
<i32 as IntoPropValue<Option<i128>>>
<i32 as IntoPropValue<Option<f64>>>
note: required by a bound in `PropsBuilder::id`
--> packages/yew/src/lib.rs:231:17
|
8 | #[derive(Clone, Properties, PartialEq)]
| ^^^^^^^^^^ required by this bound in `PropsBuilder::id`
...
11 | id: usize,
| -- required by a bound in this associated function
= note: this error originates in the derive macro `Properties` (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to previous error
For more information about this error, try `rustc --explain E0277`.
Couldn't compile the test.
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.
oh wow there are a lot of occurrences of that issue in the tests, it's terrible 😩
I'm not 100% if the pros outweigh the cons in the end...
Pros:
- basic conversion between primitives
- particularly useful if you call functions and return a value in some kind of type then need to convert
Cons:
- requires to be explicit on the type of the integers pass in props
- particularly annoying when using the values directly in props (instead of using a constant or a function)
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 will try to see if I can do something smart with the macro but I think it shouldn't go to the next release. We have enough breaking changes like that that we want to fix
Size Comparison
✅ None of the examples has changed their size significantly. |
Benchmark - SSRYew Master
Pull Request
|
Description
This change will allow the user to pass primitive values that are normally
converted easily with
.into()
in Rust.Example:
This is currently not allowed in Yew and should be implemented that way:
The use of
as
in this case is not really recommended because it is errorprone. Ideally, someone should use:
But this doesn't work either in Yew.
Checklist