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 PureComponent trait #642

Closed
wants to merge 4 commits into from

Conversation

hgzimmerman
Copy link
Member

This PR seeks to add a PureComponent trait.

This is purely an ergonomic feature that seeks to eliminate boilerplate from components that will not modify their own state. Using this trait, I saved about 25 lines over using Component when rewriting the Button component.

Instead of implementing Component, you can implement PureComponent which only requires that a render function be implemented. Optionally, if your component will communicate up the component hierarchy using callbacks, a method called emit can be overwritten.

/// Override this if the pure component will pass its messages upwards.
///
/// If the implementing struct in question has a callback, this should be overwritten.
fn emit(&self, _msg: Self::Message) { }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe there shouldn't be a default impl for this fn? Thoughts?

@hgzimmerman
Copy link
Member Author

As a possible solution to what to do about the emit fn, another trait could be specified called Emittable that could be derived by annotating the field that contains the Callback. If the annotation isn't present, then the derived get_callback function would return None.

pub trait Emittable {
    get_callback(&self) -> Option<&Callback<<Self as Component>::Message>>;
}

and the blanket impl for Component::update could look like:

// impl Component for T: PureComponent ...
    fn update(&mut self, msg: Self::Message) -> ShouldRender {
        if let Some(cb) = self.get_callback() {
            cb.emit()
        }
        false
    }

The benefit would be that PureComponent would only have one function to implement: render. No confusion about when to implement emit or having to implement an empty emit.

The drawbacks would be increased complexity, another macro derive for users to remember, and the trait bounds on PureComponent start to be a little too long with a definition like:

pub trait PureComponent: Properties + Emittable + PartialEq + Sized + 'static {...}

If a "pure component" wants to use two or more callbacks, either an EmissivePureComponent trait could be specified, which would have an emit method that must be implemented, or the Emitable::get_callback takes another parameter - a reference to the Message about to be emitted, so in a user-defined impl of Emittable, different callbacks could be selected depending on which message is being emitted.
Additionally, the derive macro might get a difficult to implement for cases with Option<Callback<_>>. The whole thing might be a little too fragile.

///
/// PureComponents should be used instead of Components if the Component in question does not mutate
/// the state passed to it via props and if it does not own services or connections to agents.
pub trait PureComponent: Properties + PartialEq + Sized + 'static {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When trait specialization arrives in a year or three, it might be possible to remove this PartialEq bound, and have blanket impls of Component for both T: PureComponent and T: PureComponent + PartialEq. The component would effectively memoize results like it does currently if the struct in question implements PartialEq, or just return true from change if it doesn't impl PartialEq.

@hgzimmerman
Copy link
Member Author

hgzimmerman commented Sep 20, 2019

As another thought: PureComponent could be implemented for any T where T: Properties + Emissive + Renderable<T>. At that point, would it make sense to even have PureComponent, except as a designation that a component is question doesn't hold state? I think there is value in keeping that around, but it certainly is debatable.

Edit:
I don't know how well that would square with moving a render method into Component like in #563.

@hgzimmerman
Copy link
Member Author

hgzimmerman commented Sep 21, 2019

A possible compatible alternative to the above comment would be:

impl <T> Renderable<T> for T where T: Component {} // from #563

impl <T> Emissive for T where T: PureComponent {...}
impl <T> PureRenderable<T> where T: PureComponent {...}
 // We need a different Renderable trait (that does the same thing) to break the trait dependency cycle.
impl <T> Component for T where T: Emissive + PureRenderable<T> + Properties + PartialEq {...}

But it would likely have to wait until #563 is complete.


Edit: forget whats above, this is probably the best representation of the idea of pure components. There isn't a need for an explicit PureComponent trait.

impl <T> Renderable<T> for T where T: Component {} // from #563
/// Can be derived
pub trait Emissive {
    type Msg;
    fn emit(&self, msg: Self::Msg);
}
pub trait PureComponent {
    fn render(&self) -> Html<Self>;
}
impl <T> Component for T where T: Emissive + PureComponent + Properties + PartialEq {...}

@jstarry
Copy link
Member

jstarry commented Sep 23, 2019

This definitely saves a lot of code which I really like! I do feel hesitant to add a new trait that is purely for ergonomics though. I think this would be a great addition to a new crate which has ergonomic syntax helpers, thoughts?

@hgzimmerman
Copy link
Member Author

hgzimmerman commented Sep 23, 2019

Since this trait uses a blanket impl <T> Component for T where T: PureComponent, orphan rules prevent that from being able to implement a trait defined in another crate.

Using that approach, the following error is produced:

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
  --> src/main.rs:79:1
   |
79 | impl <T> Component for T where T: PureComponent {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type
   |
   = note: only traits defined in the current crate can be implemented for a type parameter

So I think that something like this has to exist within yew.

Maybe feature-gating ergonomic features might be an option, but even then I would still probably want them included by default, under the philosophy that people can turn it off if they don't want that functionality. Although, under those circumstances, any dependent library (eg. router, yewstrap) would likely make use of that ergonomic improvement, so any app using those dependencies would have to use yew with the same feature set (if only as a dependency).

@hgzimmerman
Copy link
Member Author

I've come up with an alternative solution to this that can live in a separate library.
Currently, it lives here: https://github.com/hgzimmerman/yew_pure

The end result leads to usage of pure components like:

html! {
    <Pure<Button> callback=|x| {x} text = "Click me!" />
}

Needing Pure<_> to create your component in the html macro isn't ideal, but it allows the same reduction in code that this proposal does - and it allows it to live in a separate library.

Because I predict that some changes may arrive in the coming months regarding the use of messages/callbacks, I would hesitate to further entrench parts of the API that make use of it like this would.

I would like something like this to be added to the project eventually, but I think that time is later, when the API is really stable.

@hgzimmerman
Copy link
Member Author

I'm closing this for now. An approximate equivalent is available via the crate mentioned above.

@hgzimmerman hgzimmerman closed this Oct 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants