-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Redo properties, take 2 #2729
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
Redo properties, take 2 #2729
Conversation
Visit the preview URL for this PR (updated for commit 4420689): https://yew-rs-api--pr2729-prop-inheritance-939zkif9.web.app (expires Sun, 26 Jun 2022 15:29:29 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Size Comparison
✅ None of the examples has changed their size significantly. |
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.
Implementation looks good to me. I assume prop inheritance (with Deref
/DerefMut
?) will come in the future, right?
What if we create a crate like typed-builder
(similar to how #2563 creates imut)? That way a type-safe builders can be used outside of Yew as well.
6665f48
to
4420689
Compare
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.
Looks good to me!
The Deref
/DerefMut
stuff and prop inheritance can come later. If possible, we should also consider taking it out of Yew and making a separate builder crate for this
Tagged as technically breaking change, because it removes the old builder-style approach in user code. If anybody actually needs the old style, we can think of a replacement, but I think the props macro or initializing the props directly is often a good work-around. - Props::Builder::new().x(1).y(2).build()
+ props!{ Props { x: 1, y: 2 } } I have double-checked the error message stay informative in rust stable, also. I'll merge this in the evening or tomorrow. |
Description
This PR is a follow up to #2369, based on the pattern described in #2369 (comment). It solves the blocker towards inherited props by replacing the manual builder steps tracking the state of the builder (required props) with a trait-guided solution, based on the central
HasProp
andHasAllProps
traits.I suppose it would also make sense to have another look at #1634. Currently, all props are added in alphabetical order, but with the trait approach, the order shouldn't matter.
Checklist