Skip to content

Commit 5aa07d3

Browse files
committed
Fix call stack exceeded bug (#1624)
* Fix call stack exceeded bug * Fix borrow error * Update scope node_ref copy when component updates
1 parent c9189ad commit 5aa07d3

File tree

3 files changed

+49
-9
lines changed

3 files changed

+49
-9
lines changed

yew/src/html/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,19 @@ impl NodeRef {
455455
this.node = None;
456456
this.link = Some(node_ref);
457457
}
458+
459+
/// Reuse an existing `NodeRef`
460+
pub(crate) fn reuse(&self, node_ref: Self) {
461+
// Avoid circular references
462+
if self == &node_ref {
463+
return;
464+
}
465+
466+
let mut this = self.0.borrow_mut();
467+
let existing = node_ref.0.borrow();
468+
this.node = existing.node.clone();
469+
this.link = existing.link.clone();
470+
}
458471
}
459472

460473
/// Trait for rendering virtual DOM elements

yew/src/html/scope.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ pub(crate) enum ComponentUpdate<COMP: Component> {
2323
Message(COMP::Message),
2424
/// Wraps batch of messages for a component.
2525
MessageBatch(Vec<COMP::Message>),
26-
/// Wraps properties and next sibling for a component.
27-
Properties(COMP::Properties, NodeRef),
26+
/// Wraps properties, node ref, and next sibling for a component.
27+
Properties(COMP::Properties, NodeRef, NodeRef),
2828
}
2929

3030
/// Untyped scope used for accessing parent scope
@@ -391,7 +391,9 @@ where
391391
ComponentUpdate::MessageBatch(messages) => messages
392392
.into_iter()
393393
.fold(false, |acc, msg| state.component.update(msg) || acc),
394-
ComponentUpdate::Properties(props, next_sibling) => {
394+
ComponentUpdate::Properties(props, node_ref, next_sibling) => {
395+
// When components are updated, a new node ref could have been passed in
396+
state.node_ref = node_ref;
395397
// When components are updated, their siblings were likely also updated
396398
state.next_sibling = next_sibling;
397399
state.component.change(props)

yew/src/virtual_dom/vcomp.rs

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ trait Mountable {
122122
parent: Element,
123123
next_sibling: NodeRef,
124124
) -> Box<dyn Scoped>;
125-
fn reuse(self: Box<Self>, scope: &dyn Scoped, next_sibling: NodeRef);
125+
fn reuse(self: Box<Self>, node_ref: NodeRef, scope: &dyn Scoped, next_sibling: NodeRef);
126126
}
127127

128128
struct PropsWrapper<COMP: Component> {
@@ -162,9 +162,13 @@ impl<COMP: Component> Mountable for PropsWrapper<COMP> {
162162
Box::new(scope)
163163
}
164164

165-
fn reuse(self: Box<Self>, scope: &dyn Scoped, next_sibling: NodeRef) {
165+
fn reuse(self: Box<Self>, node_ref: NodeRef, scope: &dyn Scoped, next_sibling: NodeRef) {
166166
let scope: Scope<COMP> = scope.to_any().downcast();
167-
scope.update(ComponentUpdate::Properties(self.props, next_sibling));
167+
scope.update(ComponentUpdate::Properties(
168+
self.props,
169+
node_ref,
170+
next_sibling,
171+
));
168172
}
169173
}
170174

@@ -186,9 +190,9 @@ impl VDiff for VComp {
186190
if let VNode::VComp(ref mut vcomp) = &mut ancestor {
187191
// If the ancestor is the same type, reuse it and update its properties
188192
if self.type_id == vcomp.type_id && self.key == vcomp.key {
189-
self.node_ref.link(vcomp.node_ref.clone());
193+
self.node_ref.reuse(vcomp.node_ref.clone());
190194
let scope = vcomp.scope.take().expect("VComp is not mounted");
191-
mountable.reuse(scope.borrow(), next_sibling);
195+
mountable.reuse(self.node_ref.clone(), scope.borrow(), next_sibling);
192196
self.scope = Some(scope);
193197
return vcomp.node_ref.clone();
194198
}
@@ -311,14 +315,35 @@ mod tests {
311315
}
312316

313317
fn change(&mut self, _: Self::Properties) -> ShouldRender {
314-
unimplemented!();
318+
true
315319
}
316320

317321
fn view(&self) -> Html {
318322
unimplemented!();
319323
}
320324
}
321325

326+
#[test]
327+
fn update_loop() {
328+
let document = crate::utils::document();
329+
let parent_scope: AnyScope = crate::html::Scope::<Comp>::new(None).into();
330+
let parent_element = document.create_element("div").unwrap();
331+
332+
let mut ancestor = html! { <Comp></Comp> };
333+
ancestor.apply(&parent_scope, &parent_element, NodeRef::default(), None);
334+
335+
for _ in 0..10000 {
336+
let mut node = html! { <Comp></Comp> };
337+
node.apply(
338+
&parent_scope,
339+
&parent_element,
340+
NodeRef::default(),
341+
Some(ancestor),
342+
);
343+
ancestor = node;
344+
}
345+
}
346+
322347
#[test]
323348
fn set_properties_to_component() {
324349
html! {

0 commit comments

Comments
 (0)