From eb81164c11b4a0d168d8ab5abb7bc651b6a4a000 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Sun, 18 Oct 2020 18:00:22 +0800 Subject: [PATCH 1/3] Fix call stack exceeded bug --- yew/src/html/mod.rs | 8 ++++++++ yew/src/virtual_dom/vcomp.rs | 25 +++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/yew/src/html/mod.rs b/yew/src/html/mod.rs index 481a0c4a18a..3f2d1901cd8 100644 --- a/yew/src/html/mod.rs +++ b/yew/src/html/mod.rs @@ -456,6 +456,14 @@ impl NodeRef { this.node = None; this.link = Some(node_ref); } + + /// Reuse an existing `NodeRef` + pub(crate) fn reuse(&self, node_ref: Self) { + let mut this = self.0.borrow_mut(); + let existing = node_ref.0.borrow(); + this.node = existing.node.clone(); + this.link = existing.link.clone(); + } } /// Trait for building properties for a component diff --git a/yew/src/virtual_dom/vcomp.rs b/yew/src/virtual_dom/vcomp.rs index e9dcaa7d0a6..23b6d87854e 100644 --- a/yew/src/virtual_dom/vcomp.rs +++ b/yew/src/virtual_dom/vcomp.rs @@ -186,7 +186,7 @@ impl VDiff for VComp { if let VNode::VComp(ref mut vcomp) = &mut ancestor { // If the ancestor is the same type, reuse it and update its properties if self.type_id == vcomp.type_id && self.key == vcomp.key { - self.node_ref.link(vcomp.node_ref.clone()); + self.node_ref.reuse(vcomp.node_ref.clone()); let scope = vcomp.scope.take().expect("VComp is not mounted"); mountable.reuse(scope.borrow(), next_sibling); self.scope = Some(scope); @@ -316,7 +316,7 @@ mod tests { } fn change(&mut self, _: Self::Properties) -> ShouldRender { - unimplemented!(); + true } fn view(&self) -> Html { @@ -324,6 +324,27 @@ mod tests { } } + #[test] + fn update_loop() { + let document = crate::utils::document(); + let parent_scope: AnyScope = crate::html::Scope::::new(None).into(); + let parent_element = document.create_element("div").unwrap(); + + let mut ancestor = html! { }; + ancestor.apply(&parent_scope, &parent_element, NodeRef::default(), None); + + for _ in 0..10000 { + let mut node = html! { }; + node.apply( + &parent_scope, + &parent_element, + NodeRef::default(), + Some(ancestor), + ); + ancestor = node; + } + } + #[test] fn set_properties_to_component() { html! { From e1bf8be1cabaf718e99256855597b480c2579cee Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Sun, 18 Oct 2020 23:00:45 +0800 Subject: [PATCH 2/3] Fix borrow error --- yew/src/html/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/yew/src/html/mod.rs b/yew/src/html/mod.rs index 3f2d1901cd8..55653c7c468 100644 --- a/yew/src/html/mod.rs +++ b/yew/src/html/mod.rs @@ -459,6 +459,11 @@ impl NodeRef { /// Reuse an existing `NodeRef` pub(crate) fn reuse(&self, node_ref: Self) { + // Avoid circular references + if self == &node_ref { + return; + } + let mut this = self.0.borrow_mut(); let existing = node_ref.0.borrow(); this.node = existing.node.clone(); From 3ca5c71480a224690970626f78312123d1fc9d44 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Sun, 18 Oct 2020 23:46:50 +0800 Subject: [PATCH 3/3] Update scope node_ref copy when component updates --- yew/src/html/scope.rs | 8 +++++--- yew/src/virtual_dom/vcomp.rs | 12 ++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/yew/src/html/scope.rs b/yew/src/html/scope.rs index 01fd6dfc326..6c0942864d8 100644 --- a/yew/src/html/scope.rs +++ b/yew/src/html/scope.rs @@ -23,8 +23,8 @@ pub(crate) enum ComponentUpdate { Message(COMP::Message), /// Wraps batch of messages for a component. MessageBatch(Vec), - /// Wraps properties and next sibling for a component. - Properties(COMP::Properties, NodeRef), + /// Wraps properties, node ref, and next sibling for a component. + Properties(COMP::Properties, NodeRef, NodeRef), } /// Untyped scope used for accessing parent scope @@ -445,7 +445,9 @@ where ComponentUpdate::MessageBatch(messages) => messages .into_iter() .fold(false, |acc, msg| state.component.update(msg) || acc), - ComponentUpdate::Properties(props, next_sibling) => { + ComponentUpdate::Properties(props, node_ref, next_sibling) => { + // When components are updated, a new node ref could have been passed in + state.node_ref = node_ref; // When components are updated, their siblings were likely also updated state.next_sibling = next_sibling; state.component.change(props) diff --git a/yew/src/virtual_dom/vcomp.rs b/yew/src/virtual_dom/vcomp.rs index 23b6d87854e..9bbfbd62f84 100644 --- a/yew/src/virtual_dom/vcomp.rs +++ b/yew/src/virtual_dom/vcomp.rs @@ -122,7 +122,7 @@ trait Mountable { parent: Element, next_sibling: NodeRef, ) -> Box; - fn reuse(self: Box, scope: &dyn Scoped, next_sibling: NodeRef); + fn reuse(self: Box, node_ref: NodeRef, scope: &dyn Scoped, next_sibling: NodeRef); } struct PropsWrapper { @@ -162,9 +162,13 @@ impl Mountable for PropsWrapper { Box::new(scope) } - fn reuse(self: Box, scope: &dyn Scoped, next_sibling: NodeRef) { + fn reuse(self: Box, node_ref: NodeRef, scope: &dyn Scoped, next_sibling: NodeRef) { let scope: Scope = scope.to_any().downcast(); - scope.update(ComponentUpdate::Properties(self.props, next_sibling)); + scope.update(ComponentUpdate::Properties( + self.props, + node_ref, + next_sibling, + )); } } @@ -188,7 +192,7 @@ impl VDiff for VComp { if self.type_id == vcomp.type_id && self.key == vcomp.key { self.node_ref.reuse(vcomp.node_ref.clone()); let scope = vcomp.scope.take().expect("VComp is not mounted"); - mountable.reuse(scope.borrow(), next_sibling); + mountable.reuse(self.node_ref.clone(), scope.borrow(), next_sibling); self.scope = Some(scope); return vcomp.node_ref.clone(); }