diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index d6af9b051..eb0d3cc95 100644 --- a/packages/core/src/arena.rs +++ b/packages/core/src/arena.rs @@ -1,7 +1,4 @@ -use crate::{ - nodes::RenderReturn, nodes::VNode, virtual_dom::VirtualDom, AttributeValue, DynamicNode, - ScopeId, -}; +use crate::{nodes::RenderReturn, nodes::VNode, virtual_dom::VirtualDom, DynamicNode, ScopeId}; use bumpalo::boxed::Box as BumpBox; /// An Element's unique identifier. @@ -64,12 +61,11 @@ impl VirtualDom { pub(crate) fn try_reclaim(&mut self, el: ElementId) -> Option { if el.0 == 0 { panic!( - "Invalid element set to 0 - {:#?}", + "Cannot reclaim the root element - {:#?}", std::backtrace::Backtrace::force_capture() - ) + ); } - println!("reclaiming {:?}", el); self.elements.try_remove(el.0) } @@ -80,46 +76,61 @@ impl VirtualDom { // Drop a scope and all its children pub(crate) fn drop_scope(&mut self, id: ScopeId) { - let scope = self.scopes.get(id.0).unwrap(); - - if let Some(root) = scope.as_ref().try_root_node() { - let root = unsafe { root.extend_lifetime_ref() }; - if let RenderReturn::Sync(Ok(node)) = root { + if let Some(root) = self.scopes[id.0].as_ref().try_root_node() { + if let RenderReturn::Sync(Ok(node)) = unsafe { root.extend_lifetime_ref() } { self.drop_scope_inner(node) } } - let scope = self.scopes.get_mut(id.0).unwrap(); - scope.props.take(); + self.scopes[id.0].props.take(); // Drop all the hooks once the children are dropped // this means we'll drop hooks bottom-up - for hook in scope.hook_list.get_mut().drain(..) { - println!("dropping hook !"); - drop(unsafe { BumpBox::from_raw(hook) }); - println!("hook dropped !"); - } + self.scopes[id.0] + .hook_list + .get_mut() + .drain(..) + .for_each(|hook| drop(unsafe { BumpBox::from_raw(hook) })); } fn drop_scope_inner(&mut self, node: &VNode) { - for attr in node.dynamic_attrs { - if let AttributeValue::Listener(l) = &attr.value { - l.borrow_mut().take(); - } - } + node.clear_listeners(); + node.dynamic_nodes.iter().for_each(|node| match node { + DynamicNode::Component(c) => self.drop_scope(c.scope.get().unwrap()), + DynamicNode::Fragment(nodes) => nodes + .into_iter() + .for_each(|node| self.drop_scope_inner(node)), + _ => {} + }) + } - for (idx, _) in node.template.roots.iter().enumerate() { - match node.dynamic_root(idx) { - Some(DynamicNode::Component(c)) => self.drop_scope(c.scope.get().unwrap()), - Some(DynamicNode::Fragment(nodes)) => { - for node in *nodes { - self.drop_scope_inner(node); - } - } - _ => {} - } + /// Descend through the tree, removing any borrowed props and listeners + pub(crate) fn ensure_drop_safety(&self, scope: ScopeId) { + let node = unsafe { self.scopes[scope.0].previous_frame().try_load_node() }; + + // And now we want to make sure the previous frame has dropped anything that borrows self + if let Some(RenderReturn::Sync(Ok(node))) = node { + self.ensure_drop_safety_inner(node); } } + + fn ensure_drop_safety_inner(&self, node: &VNode) { + node.clear_listeners(); + + node.dynamic_nodes.iter().for_each(|child| match child { + // Only descend if the props are borrowed + DynamicNode::Component(c) if !c.static_props => { + self.ensure_drop_safety(c.scope.get().unwrap()); + c.props.set(None); + } + + DynamicNode::Fragment(f) => f + .iter() + .for_each(|node| self.ensure_drop_safety_inner(node)), + + _ => {} + }); + } } impl ElementPath { diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 27eed58d4..37e2d0d77 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -84,6 +84,14 @@ impl<'a> VNode<'a> { } } } + + pub(crate) fn clear_listeners(&self) { + for attr in self.dynamic_attrs { + if let AttributeValue::Listener(l) = &attr.value { + l.borrow_mut().take(); + } + } + } } /// A static layout of a UI tree that describes a set of dynamic and static nodes. diff --git a/packages/core/src/scope_arena.rs b/packages/core/src/scope_arena.rs index e94818fee..68aa13450 100644 --- a/packages/core/src/scope_arena.rs +++ b/packages/core/src/scope_arena.rs @@ -7,7 +7,6 @@ use crate::{ scheduler::RcWake, scopes::{ScopeId, ScopeState}, virtual_dom::VirtualDom, - AttributeValue, DynamicNode, VNode, }; use futures_util::FutureExt; use std::{ @@ -49,42 +48,6 @@ impl VirtualDom { .and_then(|id| self.scopes.get_mut(id.0).map(|f| f.as_mut() as *mut _)) } - fn ensure_drop_safety(&self, scope: ScopeId) { - let scope = &self.scopes[scope.0]; - let node = unsafe { scope.previous_frame().try_load_node() }; - - // And now we want to make sure the previous frame has dropped anything that borrows self - if let Some(RenderReturn::Sync(Ok(node))) = node { - self.ensure_drop_safety_inner(node); - } - } - - fn ensure_drop_safety_inner(&self, node: &VNode) { - for attr in node.dynamic_attrs { - if let AttributeValue::Listener(l) = &attr.value { - l.borrow_mut().take(); - } - } - - for child in node.dynamic_nodes { - match child { - DynamicNode::Component(c) => { - // Only descend if the props are borrowed - if !c.static_props { - self.ensure_drop_safety(c.scope.get().unwrap()); - c.props.set(None); - } - } - DynamicNode::Fragment(f) => { - for node in *f { - self.ensure_drop_safety_inner(node); - } - } - _ => {} - } - } - } - pub(crate) fn run_scope(&mut self, scope_id: ScopeId) -> &RenderReturn { // Cycle to the next frame and then reset it // This breaks any latent references, invalidating every pointer referencing into it. diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index bb2776861..6e048638b 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -258,8 +258,6 @@ impl<'src> ScopeState { } /// Try to retrieve a shared state with type `T` from any parent scope. - /// - /// To release the borrow, use `cloned` if the context is clone. pub fn consume_context(&self) -> Option<&T> { if let Some(this_ctx) = self.has_context() { return Some(this_ctx); @@ -307,17 +305,17 @@ impl<'src> ScopeState { pub fn provide_context(&self, value: T) -> &T { let mut contexts = self.shared_contexts.borrow_mut(); - let any = match contexts.get(&TypeId::of::()) { - Some(item) => item.downcast_ref::().unwrap() as *const T, - None => { - let boxed = Box::new(value); - let boxed_ptr = boxed.as_ref() as *const T; - contexts.insert(TypeId::of::(), boxed); - boxed_ptr - } - }; + if !contexts.contains_key(&TypeId::of::()) { + contexts.insert(TypeId::of::(), Box::new(value)); + } - unsafe { &*any } + let out = contexts + .get(&TypeId::of::()) + .unwrap() + .downcast_ref::() + .unwrap() as *const T; + + unsafe { &*out } } /// Pushes the future onto the poll queue to be polled after the component renders. diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index a7046636b..1c1d6aa15 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -629,6 +629,8 @@ impl VirtualDom { impl Drop for VirtualDom { fn drop(&mut self) { + println!("Dropping virtualdom"); + // Simply drop this scope which drops all of its children self.drop_scope(ScopeId(0)); } diff --git a/packages/core/tests/bubble_error.rs b/packages/core/tests/bubble_error.rs index 6dbd0f3f7..aa23ab718 100644 --- a/packages/core/tests/bubble_error.rs +++ b/packages/core/tests/bubble_error.rs @@ -20,9 +20,9 @@ fn app(cx: Scope) -> Element { fn it_goes() { let mut dom = VirtualDom::new(app); - let edits = dom.rebuild().santize(); + let _edits = dom.rebuild().santize(); dom.mark_dirty(ScopeId(0)); - dom.render_immediate(); + _ = dom.render_immediate(); } diff --git a/packages/core/tests/context_api.rs b/packages/core/tests/context_api.rs index 696810cb0..4040daccb 100644 --- a/packages/core/tests/context_api.rs +++ b/packages/core/tests/context_api.rs @@ -29,11 +29,11 @@ fn state_shares() { dom.mark_dirty(ScopeId(0)); _ = dom.render_immediate(); - assert_eq!(dom.base_scope().consume_context::().unwrap(), 1); + assert_eq!(dom.base_scope().consume_context::().unwrap(), &1); dom.mark_dirty(ScopeId(0)); _ = dom.render_immediate(); - assert_eq!(dom.base_scope().consume_context::().unwrap(), 2); + assert_eq!(dom.base_scope().consume_context::().unwrap(), &2); dom.mark_dirty(ScopeId(2)); assert_eq!( diff --git a/packages/core/tests/miri_full_app.rs b/packages/core/tests/miri_full_app.rs index 6cd3d2bd8..c1c6d52cc 100644 --- a/packages/core/tests/miri_full_app.rs +++ b/packages/core/tests/miri_full_app.rs @@ -8,16 +8,16 @@ fn miri_rollover() { _ = dom.rebuild(); - for x in 0..3 { + for _ in 0..3 { dom.handle_event("click", Rc::new(MouseData::default()), ElementId(2), true); dom.process_events(); - dom.render_immediate(); + _ = dom.render_immediate(); } } fn app(cx: Scope) -> Element { let mut idx = use_state(cx, || 0); - let onhover = |h| println!("go!"); + let onhover = |_| println!("go!"); cx.render(rsx! { div { @@ -31,7 +31,7 @@ fn app(cx: Scope) -> Element { button { onclick: move |_| idx -= 1, "-" } ul { (0..**idx).map(|i| rsx! { - Child { i: i, onhover: onhover } + child_example { i: i, onhover: onhover } }) } } @@ -39,7 +39,7 @@ fn app(cx: Scope) -> Element { } #[inline_props] -fn Child<'a>(cx: Scope<'a>, i: i32, onhover: EventHandler<'a, MouseEvent>) -> Element { +fn child_example<'a>(cx: Scope<'a>, i: i32, onhover: EventHandler<'a, MouseEvent>) -> Element { cx.render(rsx! { li { onmouseover: move |e| onhover.call(e), diff --git a/packages/core/tests/miri_stress.rs b/packages/core/tests/miri_stress.rs index 3ebd5c504..3105e45bc 100644 --- a/packages/core/tests/miri_stress.rs +++ b/packages/core/tests/miri_stress.rs @@ -97,7 +97,7 @@ fn memo_works_properly() { let mut dom = VirtualDom::new(app); - dom.rebuild(); + _ = dom.rebuild(); todo!() // dom.hard_diff(ScopeId(0)); // dom.hard_diff(ScopeId(0)); diff --git a/packages/core/tests/suspense.rs b/packages/core/tests/suspense.rs index 2da343707..d3fb776a9 100644 --- a/packages/core/tests/suspense.rs +++ b/packages/core/tests/suspense.rs @@ -1,9 +1,8 @@ use dioxus::core::ElementId; use dioxus::core::{Mutation::*, SuspenseContext}; use dioxus::prelude::*; -use dioxus_core::SuspenseContext; use std::future::IntoFuture; -use std::{rc::Rc, time::Duration}; +use std::time::Duration; #[tokio::test] async fn it_works() { @@ -51,7 +50,9 @@ fn app(cx: Scope) -> Element { } fn suspense_boundary(cx: Scope) -> Element { - cx.use_hook(|| cx.provide_context(Rc::new(SuspenseContext::new(cx.scope_id())))); + cx.use_hook(|| { + cx.provide_context(SuspenseContext::new(cx.scope_id())); + }); // Ensure the right types are found cx.has_context::().unwrap();