From 40a783f90927ca19bcb0d95f8d8950d55454457f Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Mon, 4 May 2020 18:30:11 +0800 Subject: [PATCH] Simplify component state --- yew-functional/src/lib.rs | 2 +- yew-functional/tests/lib.rs | 37 +++-- yew/src/html/scope.rs | 297 +++++++++++------------------------ yew/src/scheduler.rs | 5 - yew/src/virtual_dom/vcomp.rs | 8 +- yew/src/virtual_dom/vtag.rs | 21 ++- 6 files changed, 134 insertions(+), 236 deletions(-) diff --git a/yew-functional/src/lib.rs b/yew-functional/src/lib.rs index 1efe9cecf54..bf32f4579b7 100644 --- a/yew-functional/src/lib.rs +++ b/yew-functional/src/lib.rs @@ -260,7 +260,7 @@ where } use_hook( move |state: &mut UseEffectState, hook_update| { - let mut should_update = !(*state.deps == *deps); + let mut should_update = *state.deps != *deps; move || { hook_update(move |state: &mut UseEffectState| { diff --git a/yew-functional/tests/lib.rs b/yew-functional/tests/lib.rs index a9e62e8d482..322c7837397 100644 --- a/yew-functional/tests/lib.rs +++ b/yew-functional/tests/lib.rs @@ -151,10 +151,20 @@ fn use_effect_destroys_on_component_drop() { struct UseEffectFunction {} struct UseEffectWrapper {} #[derive(Properties, Clone)] - struct DestroyCalledProps { + struct WrapperProps { destroy_called: Rc, } - impl PartialEq for DestroyCalledProps { + impl PartialEq for WrapperProps { + fn eq(&self, _other: &Self) -> bool { + false + } + } + #[derive(Properties, Clone)] + struct FunctionProps { + effect_called: Rc, + destroy_called: Rc, + } + impl PartialEq for FunctionProps { fn eq(&self, _other: &Self) -> bool { false } @@ -162,15 +172,15 @@ fn use_effect_destroys_on_component_drop() { type UseEffectComponent = FunctionComponent; type UseEffectWrapperComponent = FunctionComponent; impl FunctionProvider for UseEffectFunction { - type TProps = DestroyCalledProps; + type TProps = FunctionProps; fn run(props: &Self::TProps) -> Html { + let effect_called = props.effect_called.clone(); let destroy_called = props.destroy_called.clone(); use_effect_with_deps( move |_| { - move || { - destroy_called(); - } + effect_called(); + move || destroy_called() }, (), ); @@ -178,21 +188,14 @@ fn use_effect_destroys_on_component_drop() { } } impl FunctionProvider for UseEffectWrapper { - type TProps = DestroyCalledProps; + type TProps = WrapperProps; fn run(props: &Self::TProps) -> Html { let (show, set_show) = use_state(|| true); - use_effect_with_deps( - move |_| { - set_show(false); - || {} - }, - (), - ); - if *show { + let effect_called: Rc = Rc::new(move || set_show(false)); return html! { - + }; } else { return html! { @@ -206,7 +209,7 @@ fn use_effect_destroys_on_component_drop() { let destroy_counter_c = destroy_counter.clone(); app.mount_with_props( yew::utils::document().get_element_by_id("output").unwrap(), - DestroyCalledProps { + WrapperProps { destroy_called: Rc::new(move || *destroy_counter_c.borrow_mut().deref_mut() += 1), }, ); diff --git a/yew/src/html/scope.rs b/yew/src/html/scope.rs index 63b91b44e0e..1eaf5e87f40 100644 --- a/yew/src/html/scope.rs +++ b/yew/src/html/scope.rs @@ -17,6 +17,8 @@ cfg_if! { /// Updates for a `Component` instance. Used by scope sender. pub(crate) enum ComponentUpdate { + /// Force update + Force, /// Wraps messages for a component. Message(COMP::Message), /// Wraps batch of messages for a component. @@ -28,19 +30,9 @@ pub(crate) enum ComponentUpdate { /// Untyped scope used for accessing parent scope #[derive(Debug, Clone)] pub struct AnyScope { - type_id: TypeId, - parent: Option>, - state: Rc, -} - -impl Default for AnyScope { - fn default() -> Self { - Self { - type_id: TypeId::of::<()>(), - parent: None, - state: Rc::new(()), - } - } + pub(crate) type_id: TypeId, + pub(crate) parent: Option>, + pub(crate) state: Rc, } impl From> for AnyScope { @@ -70,7 +62,7 @@ impl AnyScope { parent: self.parent, state: self .state - .downcast_ref::>>() + .downcast_ref::>>>() .expect("unexpected component type") .clone(), } @@ -80,7 +72,7 @@ impl AnyScope { /// A context which allows sending messages to a component. pub struct Scope { parent: Option>, - state: Shared>, + state: Shared>>, } impl fmt::Debug for Scope { @@ -107,14 +99,16 @@ impl Scope { /// Returns the linked component if available pub fn get_component(&self) -> Option + '_> { self.state.try_borrow().ok().and_then(|state_ref| { - state_ref.component()?; - Some(Ref::map(state_ref, |this| this.component().unwrap())) + state_ref.as_ref()?; + Some(Ref::map(state_ref, |state| { + state.as_ref().unwrap().component.as_ref() + })) }) } pub(crate) fn new(parent: Option) -> Self { let parent = parent.map(Rc::new); - let state = Rc::new(RefCell::new(ComponentState::Empty)); + let state = Rc::new(RefCell::new(None)); Scope { parent, state } } @@ -126,35 +120,25 @@ impl Scope { node_ref: NodeRef, props: COMP::Properties, ) -> Scope { - let mut scope = self; - let ready_state = ReadyState { + *self.state.borrow_mut() = Some(ComponentState::new( element, + ancestor, node_ref, - scope: scope.clone(), + self.clone(), props, - ancestor, - }; - *scope.state.borrow_mut() = ComponentState::Ready(ready_state); - scope.create(); - scope - } - - /// Schedules a task to create and render a component and then mount it to the DOM - pub(crate) fn create(&mut self) { - let state = self.state.clone(); - let create = CreateComponent { state }; - scheduler().push_comp(ComponentRunnableType::Create, Box::new(create)); - self.rendered(true); + )); + self.update(ComponentUpdate::Force, true); + self } - /// Schedules a task to send a message or new props to a component - pub(crate) fn update(&self, update: ComponentUpdate) { + /// Schedules a task to send an update to a component + pub(crate) fn update(&self, update: ComponentUpdate, first_update: bool) { let update = UpdateComponent { state: self.state.clone(), update, }; scheduler().push_comp(ComponentRunnableType::Update, Box::new(update)); - self.rendered(false); + self.rendered(first_update); } /// Schedules a task to call the rendered method on a component @@ -179,14 +163,12 @@ impl Scope { where T: Into, { - self.update(ComponentUpdate::Message(msg.into())); - self.rendered(false); + self.update(ComponentUpdate::Message(msg.into()), false); } /// Send a batch of messages to the component pub fn send_message_batch(&self, messages: Vec) { - self.update(ComponentUpdate::MessageBatch(messages)); - self.rendered(false); + self.update(ComponentUpdate::MessageBatch(messages), false); } /// Creates a `Callback` which will send a message to the linked component's @@ -234,141 +216,103 @@ impl Scope { } } -enum ComponentState { - Empty, - Ready(ReadyState), - Created(CreatedState), - Processing, - Destroyed, -} - -impl ComponentState { - fn component(&self) -> Option<&COMP> { - match self { - ComponentState::Created(state) => Some(&state.component), - _ => None, - } - } -} - -impl fmt::Display for ComponentState { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let name = match self { - ComponentState::Empty => "empty", - ComponentState::Ready(_) => "ready", - ComponentState::Created(_) => "created", - ComponentState::Processing => "processing", - ComponentState::Destroyed => "destroyed", - }; - write!(f, "{}", name) - } -} - -struct ReadyState { +struct ComponentState { element: Element, node_ref: NodeRef, - props: COMP::Properties, scope: Scope, - ancestor: Option, -} - -impl ReadyState { - fn create(self) -> CreatedState { - CreatedState { - rendered: false, - component: COMP::create(self.props, self.scope.clone()), - element: self.element, - last_frame: self.ancestor, - node_ref: self.node_ref, - scope: self.scope, - } - } -} - -struct CreatedState { + component: Box, + last_root: Option, rendered: bool, - element: Element, - component: COMP, - last_frame: Option, - node_ref: NodeRef, - scope: Scope, } -impl CreatedState { - /// Called after a component and all of its children have been rendered. - fn rendered(mut self, first_render: bool) -> Self { - self.rendered = true; - self.component.rendered(first_render); - self - } - - fn update(mut self) -> Self { - let mut root = self.component.render(); - if let Some(node) = root.apply( - &self.scope.clone().into(), - &self.element, - None, - self.last_frame, - ) { - self.node_ref.set(Some(node)); - } else if let VNode::VComp(child) = &root { - // If the root VNode is a VComp, we won't have access to the rendered DOM node - // because components render asynchronously. In order to bubble up the DOM node - // from the VComp, we need to link the currently rendering component with its - // root child component. - self.node_ref.link(child.node_ref.clone()); +impl ComponentState { + fn new( + element: Element, + ancestor: Option, + node_ref: NodeRef, + scope: Scope, + props: COMP::Properties, + ) -> Self { + let component = Box::new(COMP::create(props, scope.clone())); + Self { + element, + node_ref, + scope, + component, + last_root: ancestor, + rendered: false, } - self.last_frame = Some(root); - self } } -struct RenderedComponent +struct UpdateComponent where COMP: Component, { - state: Shared>, - first_render: bool, + state: Shared>>, + update: ComponentUpdate, } -impl Runnable for RenderedComponent +impl Runnable for UpdateComponent where COMP: Component, { fn run(self: Box) { - let current_state = self.state.replace(ComponentState::Processing); - self.state.replace(match current_state { - ComponentState::Created(s) if !s.rendered => { - ComponentState::Created(s.rendered(self.first_render)) - } - ComponentState::Destroyed | ComponentState::Created(_) => current_state, - ComponentState::Empty | ComponentState::Processing | ComponentState::Ready(_) => { - panic!("unexpected component state: {}", current_state); - } - }); + if let Some(mut state) = self.state.borrow_mut().as_mut() { + let should_update = match self.update { + ComponentUpdate::Force => true, + ComponentUpdate::Message(message) => state.component.update(message), + ComponentUpdate::MessageBatch(messages) => messages + .into_iter() + .fold(false, |acc, msg| state.component.update(msg) || acc), + ComponentUpdate::Properties(props, node_ref) => { + // When components are updated, they receive a new node ref that + // must be linked to previous one. + node_ref.link(state.node_ref.clone()); + state.component.change(props) + } + }; + + if should_update { + state.rendered = false; + let mut root = state.component.render(); + let last_root = state.last_root.take(); + if let Some(node) = + root.apply(&state.scope.clone().into(), &state.element, None, last_root) + { + state.node_ref.set(Some(node)); + } else if let VNode::VComp(child) = &root { + // If the root VNode is a VComp, we won't have access to the rendered DOM node + // because components render asynchronously. In order to bubble up the DOM node + // from the VComp, we need to link the currently rendering component with its + // root child component. + state.node_ref.link(child.node_ref.clone()); + } + state.last_root = Some(root); + }; + } } } -struct CreateComponent +struct RenderedComponent where COMP: Component, { - state: Shared>, + state: Shared>>, + first_render: bool, } -impl Runnable for CreateComponent +impl Runnable for RenderedComponent where COMP: Component, { fn run(self: Box) { - let current_state = self.state.replace(ComponentState::Processing); - self.state.replace(match current_state { - ComponentState::Ready(s) => ComponentState::Created(s.create().update()), - ComponentState::Created(_) | ComponentState::Destroyed => current_state, - ComponentState::Empty | ComponentState::Processing => { - panic!("unexpected component state: {}", current_state); + if let Some(mut state) = self.state.borrow_mut().as_mut() { + if !state.rendered { + state.rendered = true; + state.component.rendered(self.first_render); } - }); + } } } @@ -376,7 +320,7 @@ struct DestroyComponent where COMP: Component, { - state: Shared>, + state: Shared>>, } impl Runnable for DestroyComponent @@ -384,64 +328,11 @@ where COMP: Component, { fn run(self: Box) { - match self.state.replace(ComponentState::Destroyed) { - ComponentState::Created(mut this) => { - drop(this.component); - if let Some(last_frame) = &mut this.last_frame { - last_frame.detach(&this.element); - } - } - ComponentState::Ready(mut this) => { - if let Some(ancestor) = &mut this.ancestor { - ancestor.detach(&this.element); - } - } - ComponentState::Empty | ComponentState::Destroyed => {} - s @ ComponentState::Processing => panic!("unexpected component state: {}", s), - }; - } -} - -struct UpdateComponent -where - COMP: Component, -{ - state: Shared>, - update: ComponentUpdate, -} - -impl Runnable for UpdateComponent -where - COMP: Component, -{ - fn run(self: Box) { - let current_state = self.state.replace(ComponentState::Processing); - self.state.replace(match current_state { - ComponentState::Created(mut this) => { - let should_update = match self.update { - ComponentUpdate::Message(message) => this.component.update(message), - ComponentUpdate::MessageBatch(messages) => messages - .into_iter() - .fold(false, |acc, msg| this.component.update(msg) || acc), - ComponentUpdate::Properties(props, node_ref) => { - // When components are updated, they receive a new node ref that - // must be linked to previous one. - node_ref.link(this.node_ref.clone()); - this.component.change(props) - } - }; - let next_state = if should_update { - this.rendered = false; - this.update() - } else { - this - }; - ComponentState::Created(next_state) + if let Some(mut state) = self.state.borrow_mut().take() { + drop(state.component); + if let Some(last_frame) = &mut state.last_root { + last_frame.detach(&state.element); } - ComponentState::Destroyed => current_state, - ComponentState::Processing | ComponentState::Ready(_) | ComponentState::Empty => { - panic!("unexpected component state: {}", current_state); - } - }); + } } } diff --git a/yew/src/scheduler.rs b/yew/src/scheduler.rs index 5e25310f5fb..d735f31cfaf 100644 --- a/yew/src/scheduler.rs +++ b/yew/src/scheduler.rs @@ -31,7 +31,6 @@ pub(crate) struct Scheduler { pub(crate) enum ComponentRunnableType { Destroy, - Create, Update, Rendered, } @@ -40,7 +39,6 @@ pub(crate) enum ComponentRunnableType { struct ComponentScheduler { // Queues destroy: Shared>>, - create: Shared>>, update: Shared>>, // Stack @@ -51,7 +49,6 @@ impl ComponentScheduler { fn new() -> Self { ComponentScheduler { destroy: Rc::new(RefCell::new(VecDeque::new())), - create: Rc::new(RefCell::new(VecDeque::new())), update: Rc::new(RefCell::new(VecDeque::new())), rendered: Rc::new(RefCell::new(Vec::new())), } @@ -59,7 +56,6 @@ impl ComponentScheduler { fn next_runnable(&self) -> Option> { None.or_else(|| self.destroy.borrow_mut().pop_front()) - .or_else(|| self.create.borrow_mut().pop_front()) .or_else(|| self.update.borrow_mut().pop_front()) .or_else(|| self.rendered.borrow_mut().pop()) } @@ -79,7 +75,6 @@ impl Scheduler { ComponentRunnableType::Destroy => { self.component.destroy.borrow_mut().push_back(runnable) } - ComponentRunnableType::Create => self.component.create.borrow_mut().push_back(runnable), ComponentRunnableType::Update => self.component.update.borrow_mut().push_back(runnable), ComponentRunnableType::Rendered => self.component.rendered.borrow_mut().push(runnable), }; diff --git a/yew/src/virtual_dom/vcomp.rs b/yew/src/virtual_dom/vcomp.rs index 886a4542b77..7ae5cd242ed 100644 --- a/yew/src/virtual_dom/vcomp.rs +++ b/yew/src/virtual_dom/vcomp.rs @@ -129,10 +129,10 @@ impl VComp { } GeneratorType::Overwrite(any_scope) => { let mut scope: Scope = any_scope.downcast(); - scope.update(ComponentUpdate::Properties( - props.clone(), - node_ref_clone.clone(), - )); + scope.update( + ComponentUpdate::Properties(props.clone(), node_ref_clone.clone()), + false, + ); Mounted { node_ref: node_ref_clone.clone(), diff --git a/yew/src/virtual_dom/vtag.rs b/yew/src/virtual_dom/vtag.rs index bcd7c91ec55..4c770bf32cb 100644 --- a/yew/src/virtual_dom/vtag.rs +++ b/yew/src/virtual_dom/vtag.rs @@ -507,6 +507,7 @@ where mod tests { use super::*; use crate::{html, Component, ComponentLink, Html, ShouldRender}; + use std::any::TypeId; #[cfg(feature = "std_web")] use stdweb::web::{document, IElement}; #[cfg(feature = "wasm_test")] @@ -515,6 +516,14 @@ mod tests { #[cfg(feature = "wasm_test")] wasm_bindgen_test_configure!(run_in_browser); + fn test_scope() -> AnyScope { + AnyScope { + type_id: TypeId::of::<()>(), + parent: None, + state: Rc::new(()), + } + } + struct Comp; impl Component for Comp { @@ -839,7 +848,7 @@ mod tests { #[cfg(feature = "web_sys")] let document = web_sys::window().unwrap().document().unwrap(); - let scope = AnyScope::default(); + let scope = test_scope(); let div_el = document.create_element("div").unwrap(); let namespace = SVG_NAMESPACE; #[cfg(feature = "web_sys")] @@ -982,7 +991,7 @@ mod tests { #[test] fn it_does_not_set_empty_class_name() { - let scope = AnyScope::default(); + let scope = test_scope(); let parent = document().create_element("div").unwrap(); #[cfg(feature = "std_web")] @@ -999,7 +1008,7 @@ mod tests { #[test] fn it_does_not_set_missing_class_name() { - let scope = AnyScope::default(); + let scope = test_scope(); let parent = document().create_element("div").unwrap(); #[cfg(feature = "std_web")] @@ -1016,7 +1025,7 @@ mod tests { #[test] fn it_sets_class_name() { - let scope = AnyScope::default(); + let scope = test_scope(); let parent = document().create_element("div").unwrap(); #[cfg(feature = "std_web")] @@ -1072,7 +1081,7 @@ mod tests { #[test] fn swap_order_of_classes() { - let scope = AnyScope::default(); + let scope = test_scope(); let parent = document().create_element("div").unwrap(); #[cfg(feature = "std_web")] @@ -1123,7 +1132,7 @@ mod tests { #[test] fn add_class_to_the_middle() { - let scope = AnyScope::default(); + let scope = test_scope(); let parent = document().create_element("div").unwrap(); #[cfg(feature = "std_web")]