Mirrors/dioxus
2
0
Fork 0
mirror of https://github.com/DioxusLabs/dioxus synced 2024-11-23 20:53:06 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #664 from DioxusLabs/jk/fix-tests

Browse source 
fix: core tests and use stack optimization for replace + remove
This commit is contained in:
Jon Kelley 2022-12-19 11:33:25 -08:00 committed by GitHub
commit 3978bc9878
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 123 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
packages/core/Cargo.toml
View file

@ -41,6 +41,7 @@ log = "0.4.17"
[dev-dependencies]
tokio = { version = "*", features = ["full"] }
dioxus = { path = "../dioxus" }
pretty_assertions = "1.3.0"
[features]
default = []

8
packages/core/src/arena.rs
View file

@ -115,10 +115,14 @@ impl VirtualDom {
nodes.iter().for_each(|node| self.drop_scope_inner(node))
}
DynamicNode::Placeholder(t) => {
self.try_reclaim(t.id.get().unwrap());
if let Some(id) = t.id.get() {
self.try_reclaim(id);
}
}
DynamicNode::Text(t) => {
self.try_reclaim(t.id.get().unwrap());
if let Some(id) = t.id.get() {
self.try_reclaim(id);
}
}
});

154
packages/core/src/diff.rs
View file

@ -155,22 +155,7 @@ impl<'b> VirtualDom {
// Replace components that have different render fns
if left.render_fn != right.render_fn {
let created = self.create_component_node(right_template, right, idx);
let head = unsafe {
self.scopes[left.scope.get().unwrap().0]
.root_node()
.extend_lifetime_ref()
};
let last = match head {
RenderReturn::Sync(Ok(node)) => self.find_last_element(node),
_ => todo!(),
};
self.mutations.push(Mutation::InsertAfter {
id: last,
m: created,
});
self.remove_component_node(left, true);
return;
return self.replace_vcomponent(right_template, right, idx, left);
}
// Make sure the new vcomponent has the right scopeid associated to it
@ -203,6 +188,26 @@ impl<'b> VirtualDom {
});
}
fn replace_vcomponent(
&mut self,
right_template: &'b VNode<'b>,
right: &'b VComponent<'b>,
idx: usize,
left: &'b VComponent<'b>,
) {
let m = self.create_component_node(right_template, right, idx);
self.remove_component_node(left, true);
// We want to optimize the replace case to use one less mutation if possible
// Since mutations are done in reverse, the last node removed will be the first in the stack
// Instead of *just* removing it, we can use the replace mutation
match self.mutations.edits.pop().unwrap() {
Mutation::Remove { id } => self.mutations.push(Mutation::ReplaceWith { id, m }),
at => panic!("Expected remove mutation from remove_node {:#?}", at),
};
}
/// Lightly diff the two templates, checking only their roots.
///
/// The goal here is to preserve any existing component state that might exist. This is to preserve some React-like
@ -712,11 +717,21 @@ impl<'b> VirtualDom {
fn replace(&mut self, left: &'b VNode<'b>, right: impl IntoIterator<Item = &'b VNode<'b>>) {
let m = self.create_children(right);
let id = self.find_last_element(left);
self.mutations.push(Mutation::InsertAfter { id, m });
let pre_edits = self.mutations.edits.len();
self.remove_node(left, true);
// We should always have a remove mutation
// Eventually we don't want to generate placeholders, so this might not be true. But it's true today
assert!(self.mutations.edits.len() > pre_edits);
// We want to optimize the replace case to use one less mutation if possible
// Since mutations are done in reverse, the last node removed will be the first in the stack
// Instead of *just* removing it, we can use the replace mutation
match self.mutations.edits.pop().unwrap() {
Mutation::Remove { id } => self.mutations.push(Mutation::ReplaceWith { id, m }),
_ => panic!("Expected remove mutation from remove_node"),
};
}
fn node_to_placeholder(&mut self, l: &'b [VNode<'b>], r: &'b VPlaceholder) {
@ -725,24 +740,45 @@ impl<'b> VirtualDom {
r.id.set(Some(placeholder));
let id = self.find_last_element(&l[0]);
self.mutations
.push(Mutation::CreatePlaceholder { id: placeholder });
self.mutations.push(Mutation::InsertAfter { id, m: 1 });
self.remove_nodes(l);
// We want to optimize the replace case to use one less mutation if possible
// Since mutations are done in reverse, the last node removed will be the first in the stack
// Instead of *just* removing it, we can use the replace mutation
match self.mutations.edits.pop().unwrap() {
Mutation::Remove { id } => self.mutations.push(Mutation::ReplaceWith { id, m: 1 }),
_ => panic!("Expected remove mutation from remove_node"),
};
}
/// Remove these nodes from the dom
/// Wont generate mutations for the inner nodes
fn remove_nodes(&mut self, nodes: &'b [VNode<'b>]) {
nodes.iter().for_each(|node| self.remove_node(node, true));
nodes
.iter()
.rev()
.for_each(|node| self.remove_node(node, true));
}
fn remove_node(&mut self, node: &'b VNode<'b>, gen_muts: bool) {
// Clean up any attributes that have claimed a static node as dynamic for mount/unmounta
// Will not generate mutations!
self.reclaim_attributes(node);
// Remove the nested dynamic nodes
// We don't generate mutations for these, as they will be removed by the parent (in the next line)
// But we still need to make sure to reclaim them from the arena and drop their hooks, etc
self.remove_nested_dyn_nodes(node);
// Clean up the roots, assuming we need to generate mutations for these
// This is done last in order to preserve Node ID reclaim order (reclaim in reverse order of claim)
self.reclaim_roots(node, gen_muts);
}
fn reclaim_roots(&mut self, node: &VNode, gen_muts: bool) {
for (idx, _) in node.template.roots.iter().enumerate() {
if let Some(dy) = node.dynamic_root(idx) {
self.remove_dynamic_node(dy, gen_muts);
@ -754,17 +790,9 @@ impl<'b> VirtualDom {
self.reclaim(id);
}
}
}
for (idx, dyn_node) in node.dynamic_nodes.iter().enumerate() {
// Roots are cleaned up automatically above
if node.template.node_paths[idx].len() == 1 {
continue;
}
self.remove_dynamic_node(dyn_node, false);
}
// we clean up nodes with dynamic attributes, provided the node is unique and not a root node
fn reclaim_attributes(&mut self, node: &VNode) {
let mut id = None;
for (idx, attr) in node.dynamic_attrs.iter().enumerate() {
// We'll clean up the root nodes either way, so don't worry
@ -784,49 +812,69 @@ impl<'b> VirtualDom {
}
}
fn remove_nested_dyn_nodes(&mut self, node: &VNode) {
for (idx, dyn_node) in node.dynamic_nodes.iter().enumerate() {
// Roots are cleaned up automatically above
if node.template.node_paths[idx].len() == 1 {
continue;
}
self.remove_dynamic_node(dyn_node, false);
}
}
fn remove_dynamic_node(&mut self, node: &DynamicNode, gen_muts: bool) {
match node {
Component(comp) => self.remove_component_node(comp, gen_muts),
Text(t) => self.remove_text_node(t),
Placeholder(t) => self.remove_placeholder(t),
Text(t) => self.remove_text_node(t, gen_muts),
Placeholder(t) => self.remove_placeholder(t, gen_muts),
Fragment(nodes) => nodes
.iter()
.for_each(|node| self.remove_node(node, gen_muts)),
};
}
fn remove_placeholder(&mut self, t: &VPlaceholder) {
fn remove_placeholder(&mut self, t: &VPlaceholder, gen_muts: bool) {
if let Some(id) = t.id.take() {
if gen_muts {
self.mutations.push(Mutation::Remove { id });
}
self.reclaim(id)
}
}
fn remove_text_node(&mut self, t: &VText) {
fn remove_text_node(&mut self, t: &VText, gen_muts: bool) {
if let Some(id) = t.id.take() {
if gen_muts {
self.mutations.push(Mutation::Remove { id });
}
self.reclaim(id)
}
}
fn remove_component_node(&mut self, comp: &VComponent, gen_muts: bool) {
if let Some(scope) = comp.scope.take() {
match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
RenderReturn::Sync(Ok(t)) => self.remove_node(t, gen_muts),
_ => todo!("cannot handle nonstandard nodes"),
};
let scope = comp.scope.take().unwrap();
let props = self.scopes[scope.0].props.take();
match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
RenderReturn::Sync(Ok(t)) => {
println!("Removing component node sync {:?}", gen_muts);
self.remove_node(t, gen_muts)
}
_ => todo!("cannot handle nonstandard nodes"),
};
self.dirty_scopes.remove(&DirtyScope {
height: self.scopes[scope.0].height,
id: scope,
});
let props = self.scopes[scope.0].props.take();
*comp.props.borrow_mut() = unsafe { std::mem::transmute(props) };
self.dirty_scopes.remove(&DirtyScope {
height: self.scopes[scope.0].height,
id: scope,
});
// make sure to wipe any of its props and listeners
self.ensure_drop_safety(scope);
self.scopes.remove(scope.0);
}
*comp.props.borrow_mut() = unsafe { std::mem::transmute(props) };
// make sure to wipe any of its props and listeners
self.ensure_drop_safety(scope);
self.scopes.remove(scope.0);
}
fn find_first_element(&self, node: &'b VNode<'b>) -> ElementId {

10
packages/core/tests/attr_cleanup.rs
View file

@ -46,7 +46,7 @@ fn attrs_cycle() {
assert_eq!(
dom.render_immediate().santize().edits,
[
LoadTemplate { name: "template", index: 0, id: ElementId(3) },
LoadTemplate { name: "template", index: 0, id: ElementId(1) },
ReplaceWith { id: ElementId(2), m: 1 }
]
);
@ -56,10 +56,10 @@ fn attrs_cycle() {
dom.render_immediate().santize().edits,
[
LoadTemplate { name: "template", index: 0, id: ElementId(2) },
AssignId { path: &[0], id: ElementId(1) },
SetAttribute { name: "class", value: "3", id: ElementId(1), ns: None },
SetAttribute { name: "id", value: "3", id: ElementId(1), ns: None },
ReplaceWith { id: ElementId(3), m: 1 }
AssignId { path: &[0], id: ElementId(3) },
SetAttribute { name: "class", value: "3", id: ElementId(3), ns: None },
SetAttribute { name: "id", value: "3", id: ElementId(3), ns: None },
ReplaceWith { id: ElementId(1), m: 1 }
]
);

19
packages/core/tests/diff_unkeyed_list.rs
View file

@ -1,5 +1,6 @@
use dioxus::core::{ElementId, Mutation::*};
use dioxus::prelude::*;
use pretty_assertions::assert_eq;
#[test]
fn list_creates_one_by_one() {
@ -125,7 +126,7 @@ fn removes_one_by_one() {
assert_eq!(
dom.render_immediate().santize().edits,
[
CreatePlaceholder { id: ElementId(3) },
CreatePlaceholder { id: ElementId(4) },
ReplaceWith { id: ElementId(2), m: 1 }
]
);
@ -137,12 +138,12 @@ fn removes_one_by_one() {
dom.render_immediate().santize().edits,
[
LoadTemplate { name: "template", index: 0, id: ElementId(2) },
HydrateText { path: &[0], value: "0", id: ElementId(4) },
HydrateText { path: &[0], value: "0", id: ElementId(3) },
LoadTemplate { name: "template", index: 0, id: ElementId(5) },
HydrateText { path: &[0], value: "1", id: ElementId(6) },
LoadTemplate { name: "template", index: 0, id: ElementId(7) },
HydrateText { path: &[0], value: "2", id: ElementId(8) },
ReplaceWith { id: ElementId(3), m: 3 }
ReplaceWith { id: ElementId(4), m: 3 }
]
);
}
@ -264,9 +265,9 @@ fn removes_one_by_one_multiroot() {
assert_eq!(
dom.render_immediate().santize().edits,
[
Remove { id: ElementId(4) },
CreatePlaceholder { id: ElementId(5) },
ReplaceWith { id: ElementId(2), m: 1 }
CreatePlaceholder { id: ElementId(8) },
Remove { id: ElementId(2) },
ReplaceWith { id: ElementId(4), m: 1 }
]
);
}
@ -357,11 +358,11 @@ fn remove_many() {
assert_eq!(
edits.edits,
[
CreatePlaceholder { id: ElementId(11,) },
Remove { id: ElementId(9,) },
Remove { id: ElementId(7,) },
Remove { id: ElementId(5,) },
Remove { id: ElementId(1,) },
CreatePlaceholder { id: ElementId(3,) },
ReplaceWith { id: ElementId(2,), m: 1 },
]
);
@ -372,8 +373,8 @@ fn remove_many() {
edits.edits,
[
LoadTemplate { name: "template", index: 0, id: ElementId(2,) },
HydrateText { path: &[0,], value: "hello 0", id: ElementId(1,) },
ReplaceWith { id: ElementId(3,), m: 1 },
HydrateText { path: &[0,], value: "hello 0", id: ElementId(3,) },
ReplaceWith { id: ElementId(11,), m: 1 },
]
)
}