Skip to content

Commit

Permalink
Fix call stack exceeded bug (#1624)
Browse files Browse the repository at this point in the history
* Fix call stack exceeded bug

* Fix borrow error

* Update scope node_ref copy when component updates
  • Loading branch information
jstarry committed Oct 18, 2020
1 parent c9189ad commit 5aa07d3
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 9 deletions.
13 changes: 13 additions & 0 deletions yew/src/html/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,19 @@ impl NodeRef {
this.node = None;
this.link = Some(node_ref);
}

/// 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();
this.link = existing.link.clone();
}
}

/// Trait for rendering virtual DOM elements
Expand Down
8 changes: 5 additions & 3 deletions yew/src/html/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ pub(crate) enum ComponentUpdate<COMP: Component> {
Message(COMP::Message),
/// Wraps batch of messages for a component.
MessageBatch(Vec<COMP::Message>),
/// 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
Expand Down Expand Up @@ -391,7 +391,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)
Expand Down
37 changes: 31 additions & 6 deletions yew/src/virtual_dom/vcomp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ trait Mountable {
parent: Element,
next_sibling: NodeRef,
) -> Box<dyn Scoped>;
fn reuse(self: Box<Self>, scope: &dyn Scoped, next_sibling: NodeRef);
fn reuse(self: Box<Self>, node_ref: NodeRef, scope: &dyn Scoped, next_sibling: NodeRef);
}

struct PropsWrapper<COMP: Component> {
Expand Down Expand Up @@ -162,9 +162,13 @@ impl<COMP: Component> Mountable for PropsWrapper<COMP> {
Box::new(scope)
}

fn reuse(self: Box<Self>, scope: &dyn Scoped, next_sibling: NodeRef) {
fn reuse(self: Box<Self>, node_ref: NodeRef, scope: &dyn Scoped, next_sibling: NodeRef) {
let scope: Scope<COMP> = scope.to_any().downcast();
scope.update(ComponentUpdate::Properties(self.props, next_sibling));
scope.update(ComponentUpdate::Properties(
self.props,
node_ref,
next_sibling,
));
}
}

Expand All @@ -186,9 +190,9 @@ 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);
mountable.reuse(self.node_ref.clone(), scope.borrow(), next_sibling);
self.scope = Some(scope);
return vcomp.node_ref.clone();
}
Expand Down Expand Up @@ -311,14 +315,35 @@ mod tests {
}

fn change(&mut self, _: Self::Properties) -> ShouldRender {
unimplemented!();
true
}

fn view(&self) -> Html {
unimplemented!();
}
}

#[test]
fn update_loop() {
let document = crate::utils::document();
let parent_scope: AnyScope = crate::html::Scope::<Comp>::new(None).into();
let parent_element = document.create_element("div").unwrap();

let mut ancestor = html! { <Comp></Comp> };
ancestor.apply(&parent_scope, &parent_element, NodeRef::default(), None);

for _ in 0..10000 {
let mut node = html! { <Comp></Comp> };
node.apply(
&parent_scope,
&parent_element,
NodeRef::default(),
Some(ancestor),
);
ancestor = node;
}
}

#[test]
fn set_properties_to_component() {
html! {
Expand Down

0 comments on commit 5aa07d3

Please sign in to comment.