Apply suggestions from code review (part 2)

yewstack · Jul 22, 2021 · 7f2acd5 · 7f2acd5
1 parent 6ed6467
commit 7f2acd5
Show file tree

Hide file tree

Showing 14 changed files with 71 additions and 138 deletions.
diff --git a/examples/keyed_list/src/person.rs b/examples/keyed_list/src/person.rs
@@ -46,7 +46,7 @@ impl PersonInfo {
     }
 }
 
-#[derive(Clone, Debug, Eq, PartialEq, Properties)]
+#[derive(Debug, Eq, PartialEq, Properties)]
 pub struct PersonProps {
     info: PersonInfo,
 }

diff --git a/packages/yew/src/html/component/children.rs b/packages/yew/src/html/component/children.rs
@@ -123,9 +123,8 @@ pub type Children = ChildrenRenderer<Html>;
 ///#   fn view(&self, ctx: &Context<Self>) -> Html {
 ///         html!{{
 ///             for ctx.props().children.iter().map(|mut item| {
-///                 let mut props = (*item.props).clone();
-///                 props.value = format!("item-{}", item.props.value);
-///                 item.props = Rc::new(props);
+///                 let mut props = Rc::make_mut(&mut item.props);
+///                 props.value = format!("item-{}", props.value);
 ///                 item
 ///             })
 ///         }}

diff --git a/website/docs/advanced-topics/optimizations.md b/website/docs/advanced-topics/optimizations.md
@@ -4,42 +4,6 @@ sidebar_label: Optimizations
 description: "Make your app faster"
 ---
 
-## neq\_assign
-
-When a component receives props from its parent component, the `change` method is called. This, in 
-addition to allowing you to update the component's state, also allows you to return a `ShouldRender` 
-boolean value that indicates if the component should re-render itself in response to the prop changes.
-
-Re-rendering is expensive, and if you can avoid it, you should. As a general rule, you only want to 
-re-render when the props actually changed. The following block of code represents this rule, returning 
-`true` if the props differed from the previous props:
-
-```rust
-use yew::ShouldRender;
-
-#[derive(PartialEq)]
-struct ExampleProps;
-
-struct Example {
-    props: ExampleProps,
-};
-
-impl Example {
-    fn change(&mut self, props: ExampleProps) -> ShouldRender {
-        if self.props != props {
-            self.props = props;
-            true
-        } else {
-            false
-        }
-    }
-}
-```
-
-But we can go further! This is six lines of boilerplate can be reduced down to one by using a trait 
-and a blanket implementation for anything that implements `PartialEq`. Check out the [`NeqAssign` trait](https://docs.rs/yew/*/yew/utils/trait.NeqAssign.html) which implements
-this.
-
 ## Using smart pointers effectively
 
 **Note: if you're unsure about some of the terms used in this section, the Rust Book has a useful
@@ -51,7 +15,7 @@ references to the relevant data in your props and child components instead of th
 can avoid cloning any data until you need to modify it in the child component, where you can 
 use `Rc::make_mut` to clone and obtain a mutable reference to the data you want to alter. 
 
-This brings further benefits in `Component::change` when working out whether prop changes require 
+This brings further benefits in `Component::changed` when working out whether prop changes require 
 the component to re-render. This is because instead of comparing the value of the data the 
 underlying pointer addresses (i.e. the position in a machine's memory where the data is stored) can 
 instead be compared; if two pointers point to the same data then the value of the data they point to 
@@ -71,6 +35,10 @@ This optimization works best if the values are never updated by the children, an
 they are rarely updated by parents. This makes `Rc<_>s` a good choice for wrapping property values 
 in for pure components.
 
+However, it must be noted that unless you need to clone the data yourself in the child component, 
+this optimization is not only useless, it also adds unnecessary cost of reference counting. Props
+in Yew are already reference counted and no data clones occur internally.
+
 ## View functions
 
 For code readability reasons, it often makes sense to migrate sections of `html!` to their own 
@@ -88,11 +56,7 @@ instead of expression syntax \(`{some_view_function()}`\), and that depending on
 implementation, they can be memoized (this means that once a function is called its value is "saved"
 so that if it's called with the same arguments more than once it doesn't have to recompute its value
 and can just return the saved value from the first function call) - preventing re-renders for 
-identical props using the aforementioned `neq_assign` logic.
-
-Yew doesn't natively support pure or function components, but they are available via external crates.
-
-## Keyed DOM nodes when they arrive
+identical props. Yew compares the props internally and so the UI is only re-rendered if the props change.
 
 ## Reducing compile time using workspaces
 

diff --git a/website/docs/concepts/components.md b/website/docs/concepts/components.md
@@ -25,17 +25,14 @@ It is common to store the props (data which can be passed from parent to child c
 `ComponentLink` in your component struct, like so:
 
 ```rust
-pub struct MyComponent {
-    props: Props,
-    link: ComponentLink<Self>,
-}
+pub struct MyComponent;
 
 impl Component for MyComponent {
     type Properties = Props;
     // ...
 
-    fn create(props: Self::Properties, link: ComponentLink<Self>) -> Self {
-        MyComponent { props, link }
+    fn create(ctx: &Context<Self>) -> Self {
+        MyComponent
     }
 
     // ...
@@ -54,8 +51,8 @@ differences in programming language aside).
 impl Component for MyComponent {
     // ...
 
-    fn view(&self) -> Html {
-        let onclick = self.link.callback(|_| Msg::Click);
+    fn view(&self, ctx: &Context<Self>) -> Html {
+        let onclick = ctx.link().callback(|_| Msg::Click);
         html! {
             <button onclick={onclick}>{ self.props.button_text }</button>
         }
@@ -84,13 +81,13 @@ pub struct MyComponent {
 impl Component for MyComponent {
     // ...
 
-    fn view(&self) -> Html {
+    fn view(&self, ctx: &Context<Self>) -> Html {
         html! {
             <input ref={self.node_ref.clone()} type="text" />
         }
     }
 
-    fn rendered(&mut self, first_render: bool) {
+    fn rendered(&mut self, _ctx: &Context<Self>, first_render: bool) {
         if first_render {
             if let Some(input) = self.node_ref.cast::<HtmlInputElement>() {
                 input.focus();
@@ -123,7 +120,7 @@ impl Component for MyComponent {
 
     // ...
 
-    fn update(&mut self, msg: Self::Message) -> ShouldRender {
+    fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> ShouldRender {
        match msg {
            Msg::SetInputEnabled(enabled) => {
                if self.input_enabled != enabled {
@@ -198,4 +195,10 @@ enum Msg {
 }
 ```
 
-`Properties` represents the information passed to a component from its parent. This type must implements the `Properties` trait \(usually by deriving it\) and can specify whether certain properties are required or optional. This type is used when creating and updating a component. It is common practice to create a struct called `Props` in your component's module and use that as the component's `Properties` type. It is common to shorten "properties" to "props". Since props are handed down from parent components, the root component of your application typically has a `Properties` type of `()`. If you wish to specify properties for your root component, use the `App::mount_with_props` method.
+`Properties` represents the information passed to a component from its parent. This type must implement the `Properties` trait \(usually by deriving it\) and can specify whether certain properties are required or optional. This type is used when creating and updating a component. It is common practice to create a struct called `Props` in your component's module and use that as the component's `Properties` type. It is common to shorten "properties" to "props". Since props are handed down from parent components, the root component of your application typically has a `Properties` type of `()`. If you wish to specify properties for your root component, use the `App::mount_with_props` method.
+
+## Context
+
+All component lifecycle methods take a context object. This object provides a reference to component's scope, which
+allows sending messages to a component and the props passed to the component.
+
diff --git a/website/docs/concepts/components/callbacks.md b/website/docs/concepts/components/callbacks.md
@@ -70,7 +70,7 @@ They have an `emit` function that takes their `<IN>` type as an argument and con
 A simple use of a callback might look something like this:
 
 ```rust
-let onclick = self.link.callback(|_| Msg::Clicked);
+let onclick = link.callback(|_| Msg::Clicked);
 html! {
     <button onclick={onclick}>{ "Click" }</button>
 }
@@ -81,7 +81,7 @@ The function passed to `callback` must always take a parameter. For example, the
 If you need a callback that might not need to cause an update, use `batch_callback`.
 
 ```rust
-let onkeypress = self.link.batch_callback(|event| {
+let onkeypress = link.batch_callback(|event| {
     if event.key() == "Enter" {
         Some(Msg::Submit)
     } else {

diff --git a/website/docs/concepts/components/children.md b/website/docs/concepts/components/children.md
@@ -17,18 +17,16 @@ pub struct ListProps {
     pub children: Children,
 }
 
-pub struct List {
-    props: ListProps,
-}
+pub struct List;
 
 impl Component for List {
     type Properties = ListProps;
     // ...
 
-    fn view(&self) -> Html {
+    fn view(&self, ctx: &Context<Self>) -> Html {
         html! {
             <div class="list">
-                { for self.props.children.iter() }
+                { for ctx.props().children.iter() }
             </div>
         }
     }
@@ -53,18 +51,16 @@ pub struct ListProps {
     pub children: ChildrenWithProps<Item>,
 }
 
-pub struct List {
-    props: ListProps,
-}
+pub struct List;
 
 impl Component for ListProps {
     type Properties = ListProps;
     // ...
 
-    fn view(&self) -> Html {
+    fn view(&self, ctx: &Context<Self>) -> Html {
         html! {
             <div class="list">
-                { for self.props.children.iter() }
+                { for ctx.props().children.iter() }
             </div>
         }
     }
@@ -108,18 +104,16 @@ pub struct ListProps {
     pub children: ChildrenRenderer<Item>,
 }
 
-pub struct List {
-    props: ListProps,
-}
+pub struct List;
 
 impl Component for List {
     type Properties = ListProps;
     // ...
 
-    fn view(&self) -> Html {
+    fn view(&self, ctx: &Context<Self>) -> Html {
         html! {
             <div class="list">
-                { for self.props.children.iter() }
+                { for ctx.props().children.iter() }
             </div>
         }
     }
@@ -140,18 +134,16 @@ pub struct PageProps {
     pub sidebar: Option<VChild<PageSideBar>>,
 }
 
-struct Page {
-    props: PageProps,
-}
+struct Page;
 
 impl Component for Page {
     type Properties = PageProps;
     // ...
 
-    fn view(&self) -> Html {
+    fn view(&self, ctx: &Context<Self>) -> Html {
         html! {
             <div class="page">
-                { self.props.sidebar.clone().map(Html::from).unwrap_or_default() }
+                { ctx.props().sidebar.clone().map(Html::from).unwrap_or_default() }
                 // ... page content
             </div>
         }

diff --git a/website/docs/concepts/components/properties.md b/website/docs/concepts/components/properties.md
@@ -12,7 +12,7 @@ reason for it to be anything but a struct where each field represents a property
 
 Instead of implementing the `Properties` trait yourself, you should use `#[derive(Properties)]` to
 automatically generate the implementation instead.
-Types for which you derive `Properties` must also implement `Clone`.
+Types for which you derive `Properties` must also implement `PartialEq`.
 
 ### Field attributes
 

diff --git a/website/docs/concepts/contexts.md b/website/docs/concepts/contexts.md
@@ -49,19 +49,17 @@ The children are re-rendered when the context changes.
 
 #### Struct components
 
-The `ComponentLink::context` method is used to consume contexts in struct components.
+The `Scope::context` method is used to consume contexts in struct components.
 
 ##### Example
 
 ```rust
-struct ContextDemo {
-    link: ComponentLink<Self> 
-}
+struct ContextDemo;
 
 impl Component for ContextDemo {
     /// ...
-    fn view(&self) -> Html {
-        let theme = self.link.context::<Theme>();
+    fn view(&self, ctx: &Context<Self>) -> Html {
+        let theme = ctx.link().context::<Theme>();
         html! {
             <button style={format!("background: {}; color: {};", theme.background, theme.foreground)}>
                 { "Click me!" }

diff --git a/website/docs/concepts/function-components/attribute.md b/website/docs/concepts/function-components/attribute.md
@@ -21,7 +21,7 @@ html! { <ChatContainer /> }
 <!--With props-->
 
 ```rust
-#[derive(Properties, Clone, PartialEq)]
+#[derive(Properties, PartialEq)]
 pub struct RenderedAtProps {
     pub time: String,
 }
@@ -68,16 +68,16 @@ fn app() -> Html {
 The `#[function_component(_)]` attribute also works with generic functions for creating generic components.
 
 ```rust
-#[derive(Properties, Clone, PartialEq)]
+#[derive(Properties, PartialEq)]
 pub struct Props<T>
-    where T: Clone + PartialEq
+    where T: PartialEq
 {
     data: T,
 }
 
 #[function_component(MyGenericComponent)]
 pub fn my_generic_component<T>(props: &Props<T>) -> Html
-    where T: Clone + PartialEq + Display
+    where T: PartialEq + Display
 {
     html! {
         <p>

diff --git a/website/docs/concepts/html/classes.md b/website/docs/concepts/html/classes.md
@@ -79,29 +79,27 @@ html! {
 ```rust
 use boolinator::Boolinator;
 
-#[derive(Clone, Properties)]
+#[derive(Properties)]
 struct Props {
     #[prop_or_default]
     class: Classes,
     fill: bool,
     children: Children,
 }
 
-struct MyComponent {
-    props: Props,
-}
+struct MyComponent;
 
 impl Component for MyComponent {
     type Properties = Props;
 
     // ...
 
-    fn view(&self) -> Html {
+    fn view(&self, ctx: &Context<Self>) -> Html {
         let Props {
             class,
             fill,
             children,
-        } = &self.props;
+        } = &ctx.props();
         html! {
             <div
                 class={classes!(