From 958c02a26e86b07431d3e5f51f00afe116656981 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Thu, 2 Sep 2021 00:10:09 -0400 Subject: [PATCH] wip: working down warnings --- packages/core/src/bumpframe.rs | 4 --- packages/core/src/diff.rs | 60 +++++++++++++++++---------------- packages/core/src/lib.rs | 3 +- packages/core/src/scope.rs | 16 ++++----- packages/core/src/test_dom.rs | 4 +-- packages/core/tests/diffing.rs | 61 ++++++++++++++++++---------------- 6 files changed, 77 insertions(+), 71 deletions(-) diff --git a/packages/core/src/bumpframe.rs b/packages/core/src/bumpframe.rs index b2b54107e..8cb6d6734 100644 --- a/packages/core/src/bumpframe.rs +++ b/packages/core/src/bumpframe.rs @@ -40,10 +40,6 @@ impl ActiveFrame { self.wip_frame_mut().bump.reset() } - pub fn update_head_node<'a>(&mut self, node: VNode<'a>) { - self.wip_frame_mut().head_node = unsafe { std::mem::transmute(node) }; - } - /// The "work in progress frame" represents the frame that is currently being worked on. pub fn wip_frame(&self) -> &BumpFrame { match self.generation.get() & 1 == 0 { diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 75d1d9a9a..bafdfea35 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -294,15 +294,18 @@ impl<'bump> DiffMachine<'bump> { self.stack.add_child_count(1); - let cur_scope_id = self.stack.current_scope().unwrap(); - let scope = self.vdom.get_scope(cur_scope_id).unwrap(); + if let Some(cur_scope_id) = self.stack.current_scope() { + let scope = self.vdom.get_scope(cur_scope_id).unwrap(); - listeners.iter().for_each(|listener| { - self.attach_listener_to_scope(listener, scope); - listener.mounted_node.set(Some(real_id)); - self.mutations - .new_event_listener(listener, cur_scope_id.clone()); - }); + listeners.iter().for_each(|listener| { + self.attach_listener_to_scope(listener, scope); + listener.mounted_node.set(Some(real_id)); + self.mutations + .new_event_listener(listener, cur_scope_id.clone()); + }); + } else { + log::warn!("create element called with no scope on the stack - this is an error for a live dom"); + } for attr in *attributes { self.mutations.set_attribute(attr); @@ -465,28 +468,29 @@ impl<'bump> DiffMachine<'bump> { // // TODO: take a more efficient path than this - let cur_scope_id = self.stack.current_scope().unwrap(); - let scope = self.vdom.get_scope(cur_scope_id).unwrap(); + if let Some(cur_scope_id) = self.stack.current_scope() { + let scope = self.vdom.get_scope(cur_scope_id).unwrap(); - if old.listeners.len() == new.listeners.len() { - for (old_l, new_l) in old.listeners.iter().zip(new.listeners.iter()) { - if old_l.event != new_l.event { - please_commit(&mut self.mutations.edits); - self.mutations.remove_event_listener(old_l.event); - self.mutations.new_event_listener(new_l, cur_scope_id); + if old.listeners.len() == new.listeners.len() { + for (old_l, new_l) in old.listeners.iter().zip(new.listeners.iter()) { + if old_l.event != new_l.event { + please_commit(&mut self.mutations.edits); + self.mutations.remove_event_listener(old_l.event); + self.mutations.new_event_listener(new_l, cur_scope_id); + } + new_l.mounted_node.set(old_l.mounted_node.get()); + self.attach_listener_to_scope(new_l, scope); + } + } else { + please_commit(&mut self.mutations.edits); + for listener in old.listeners { + self.mutations.remove_event_listener(listener.event); + } + for listener in new.listeners { + listener.mounted_node.set(Some(root)); + self.mutations.new_event_listener(listener, cur_scope_id); + self.attach_listener_to_scope(listener, scope); } - new_l.mounted_node.set(old_l.mounted_node.get()); - self.attach_listener_to_scope(new_l, scope); - } - } else { - please_commit(&mut self.mutations.edits); - for listener in old.listeners { - self.mutations.remove_event_listener(listener.event); - } - for listener in new.listeners { - listener.mounted_node.set(Some(root)); - self.mutations.new_event_listener(listener, cur_scope_id); - self.attach_listener_to_scope(listener, scope); } } diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index 8193673e4..22fa4f09a 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -45,6 +45,7 @@ pub(crate) mod innerlude { pub use crate::nodes::*; pub use crate::scheduler::*; pub use crate::scope::*; + pub use crate::test_dom::*; pub use crate::util::*; pub use crate::virtual_dom::*; @@ -57,7 +58,7 @@ pub(crate) mod innerlude { pub use crate::innerlude::{ format_args_f, html, rsx, Context, DiffInstruction, DioxusElement, DomEdit, DomTree, ElementId, EventPriority, LazyNodes, MountType, Mutations, NodeFactory, Properties, ScopeId, - SuspendedContext, SyntheticEvent, UiEvent, VNode, VirtualDom, FC, + SuspendedContext, SyntheticEvent, TestDom, UiEvent, VNode, VirtualDom, FC, }; pub mod prelude { diff --git a/packages/core/src/scope.rs b/packages/core/src/scope.rs index 9b154f097..0f5cae2c7 100644 --- a/packages/core/src/scope.rs +++ b/packages/core/src/scope.rs @@ -62,7 +62,7 @@ impl Scope { // // Scopes cannot be made anywhere else except for this file // Therefore, their lifetimes are connected exclusively to the virtual dom - pub fn new<'creator_node>( + pub(crate) fn new<'creator_node>( caller: Rc, arena_idx: ScopeId, parent: Option, @@ -211,15 +211,11 @@ impl Scope { } } - pub fn root(&self) -> &VNode { - self.frames.fin_head() - } - - pub fn child_nodes<'a>(&'a self) -> ScopeChildren { + pub(crate) fn child_nodes<'a>(&'a self) -> ScopeChildren { unsafe { self.child_nodes.shorten_lifetime() } } - pub fn call_suspended_node<'a>(&'a self, task: u64) { + pub(crate) fn call_suspended_node<'a>(&'a self, task: u64) { let g = self.suspended_nodes.borrow_mut(); if let Some(suspended) = g.get(&task) { @@ -239,7 +235,7 @@ impl Scope { } } - pub fn consume_garbage(&self) -> Vec<&VNode> { + pub(crate) fn consume_garbage(&self) -> Vec<&VNode> { self.pending_garbage .borrow_mut() .drain(..) @@ -251,4 +247,8 @@ impl Scope { }) .collect::>() } + + pub fn root(&self) -> &VNode { + self.frames.fin_head() + } } diff --git a/packages/core/src/test_dom.rs b/packages/core/src/test_dom.rs index 4ab761fbb..39b9e5e74 100644 --- a/packages/core/src/test_dom.rs +++ b/packages/core/src/test_dom.rs @@ -12,9 +12,10 @@ pub struct TestDom { impl TestDom { pub fn new() -> TestDom { let bump = Bump::new(); - let scheduler = Scheduler::new(); + let mut scheduler = Scheduler::new(); TestDom { bump, scheduler } } + pub fn new_factory<'a>(&'a self) -> NodeFactory<'a> { NodeFactory::new(&self.bump) } @@ -30,7 +31,6 @@ impl TestDom { let mutations = Mutations::new(); let mut machine = DiffMachine::new(mutations, &self.scheduler.pool); machine.stack.push(DiffInstruction::DiffNode { new, old }); - machine.mutations } diff --git a/packages/core/tests/diffing.rs b/packages/core/tests/diffing.rs index c6cfdd5f1..2f58eb0ae 100644 --- a/packages/core/tests/diffing.rs +++ b/packages/core/tests/diffing.rs @@ -1,13 +1,13 @@ //! Diffing Tests +//! +//! These tests only verify that the diffing algorithm works properly for single components. +//! +//! It does not validated that component lifecycles work properly. This is done in another test file. -use bumpalo::Bump; - -use dioxus::{ - diff::DiffMachine, prelude::*, scheduler::Scheduler, DiffInstruction, DomEdit, MountType, -}; +use dioxus::{prelude::*, DomEdit, TestDom}; use dioxus_core as dioxus; use dioxus_html as dioxus_elements; -use futures_util::FutureExt; + mod test_logging; use DomEdit::*; @@ -15,13 +15,18 @@ use DomEdit::*; // feel free to enable while debugging const IS_LOGGING_ENABLED: bool = false; +fn new_dom() -> TestDom { + test_logging::set_up_logging(IS_LOGGING_ENABLED); + TestDom::new() +} + #[test] fn diffing_works() {} /// Should push the text node onto the stack and modify it #[test] fn html_and_rsx_generate_the_same_output() { - let dom = TestDom::new(); + let dom = new_dom(); let (create, change) = dom.lazy_diff( rsx! ( div { "Hello world" } ), rsx! ( div { "Goodbye world" } ), @@ -54,7 +59,7 @@ fn html_and_rsx_generate_the_same_output() { /// Should result in 3 elements on the stack #[test] fn fragments_create_properly() { - let dom = TestDom::new(); + let dom = new_dom(); let create = dom.create(rsx! { div { "Hello a" } @@ -91,7 +96,7 @@ fn fragments_create_properly() { /// Should result in the creation of an anchor (placeholder) and then a replacewith #[test] fn empty_fragments_create_anchors() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ (0..0).map(|f| rsx! { div {}}) }); let right = rsx!({ (0..1).map(|f| rsx! { div {}}) }); @@ -114,7 +119,7 @@ fn empty_fragments_create_anchors() { /// Should result in the creation of an anchor (placeholder) and then a replacewith m=5 #[test] fn empty_fragments_create_many_anchors() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ (0..0).map(|f| rsx! { div {}}) }); let right = rsx!({ (0..5).map(|f| rsx! { div {}}) }); @@ -141,7 +146,7 @@ fn empty_fragments_create_many_anchors() { /// Includes child nodes inside the fragment #[test] fn empty_fragments_create_anchors_with_many_children() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ (0..0).map(|f| rsx! { div {} }) }); let right = rsx!({ @@ -184,7 +189,7 @@ fn empty_fragments_create_anchors_with_many_children() { /// Should result in every node being pushed and then replaced with an anchor #[test] fn many_items_become_fragment() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ (0..2).map(|f| { @@ -227,7 +232,7 @@ fn many_items_become_fragment() { /// Should result in no edits #[test] fn two_equal_fragments_are_equal() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ (0..2).map(|f| { @@ -247,7 +252,7 @@ fn two_equal_fragments_are_equal() { /// Should result the creation of more nodes appended after the old last node #[test] fn two_fragments_with_differrent_elements_are_differet() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!( { (0..2).map(|_| rsx! { div { }} ) } @@ -280,7 +285,7 @@ fn two_fragments_with_differrent_elements_are_differet() { /// Should result in multiple nodes destroyed - with changes to the first nodes #[test] fn two_fragments_with_differrent_elements_are_differet_shorter() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!( {(0..5).map(|f| {rsx! { div { }}})} @@ -321,7 +326,7 @@ fn two_fragments_with_differrent_elements_are_differet_shorter() { /// Should result in multiple nodes destroyed - with no changes #[test] fn two_fragments_with_same_elements_are_differet() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!( {(0..2).map(|f| {rsx! { div { }}})} @@ -356,7 +361,7 @@ fn two_fragments_with_same_elements_are_differet() { /// should result in the removal of elements #[test] fn keyed_diffing_order() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!( {(0..5).map(|f| {rsx! { div { key: "{f}" }}})} @@ -377,7 +382,7 @@ fn keyed_diffing_order() { /// Should result in moves, but not removals or additions #[test] fn keyed_diffing_out_of_order() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [0, 1, 2, 3, /**/ 4, 5, 6, /**/ 7, 8, 9].iter().map(|f| { @@ -402,7 +407,7 @@ fn keyed_diffing_out_of_order() { /// Should result in moves only #[test] fn keyed_diffing_out_of_order_adds() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [/**/ 4, 5, 6, 7, 8 /**/].iter().map(|f| { @@ -429,7 +434,7 @@ fn keyed_diffing_out_of_order_adds() { /// Should result in moves onl #[test] fn keyed_diffing_out_of_order_adds_2() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [/**/ 4, 5, 6, 7, 8 /**/].iter().map(|f| { @@ -457,7 +462,7 @@ fn keyed_diffing_out_of_order_adds_2() { /// Should result in moves onl #[test] fn keyed_diffing_out_of_order_adds_3() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [/**/ 4, 5, 6, 7, 8 /**/].iter().map(|f| { @@ -485,7 +490,7 @@ fn keyed_diffing_out_of_order_adds_3() { /// Should result in moves onl #[test] fn keyed_diffing_out_of_order_adds_4() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [/**/ 4, 5, 6, 7, 8 /**/].iter().map(|f| { @@ -513,7 +518,7 @@ fn keyed_diffing_out_of_order_adds_4() { /// Should result in moves onl #[test] fn keyed_diffing_out_of_order_adds_5() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [/**/ 4, 5, 6, 7, 8 /**/].iter().map(|f| { @@ -536,7 +541,7 @@ fn keyed_diffing_out_of_order_adds_5() { #[test] fn keyed_diffing_additions() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [/**/ 4, 5, 6, 7, 8 /**/].iter().map(|f| { @@ -563,7 +568,7 @@ fn keyed_diffing_additions() { #[test] fn keyed_diffing_additions_and_moves_on_ends() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [/**/ 4, 5, 6, 7 /**/].iter().map(|f| { @@ -595,7 +600,7 @@ fn keyed_diffing_additions_and_moves_on_ends() { #[test] fn keyed_diffing_additions_and_moves_in_middle() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [/**/ 4, 5, 6, 7 /**/].iter().map(|f| { @@ -632,7 +637,7 @@ fn keyed_diffing_additions_and_moves_in_middle() { #[test] fn controlled_keyed_diffing_out_of_order() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [4, 5, 6, 7].iter().map(|f| { @@ -669,7 +674,7 @@ fn controlled_keyed_diffing_out_of_order() { #[test] fn controlled_keyed_diffing_out_of_order_max_test() { - let dom = TestDom::new(); + let dom = new_dom(); let left = rsx!({ [0, 1, 2, 3, 4].iter().map(|f| {