Fix side-by-side fragments by walking in reverse (#2841)
* got a repro, but no solution :(
* hmmmmm 🤔
* awyayayayayayya
* omg it's because we dont create in rev
* Make nodes rev
* fix tests compile
* Clean up PR
* revert changes to native
* Clean up PR, add test
This commit is contained in:
1 parent
851abe82d5
commit
2f30c73678
4 files changed
+184
-180
No files matched your search
@@ -694,9 +694,50 @@ impl VNode {
|
||||
dom: &mut VirtualDom,
|
||||
mut to: Option<&mut impl WriteMutations>,
|
||||
) {
|
||||
// Only take nodes that are under this root node
|
||||
let from_root_node = |(_, path): &(usize, &[u8])| path.first() == Some(&root_idx);
|
||||
while let Some((dynamic_node_id, path)) = dynamic_nodes_iter.next_if(from_root_node) {
|
||||
fn collect_dyn_node_range(
|
||||
dynamic_nodes: &mut Peekable<impl Iterator<Item = (usize, &'static [u8])>>,
|
||||
root_idx: u8,
|
||||
) -> Option<(usize, usize)> {
|
||||
let start = match dynamic_nodes.peek() {
|
||||
Some((idx, [first, ..])) if *first == root_idx => *idx,
|
||||
_ => return None,
|
||||
};
|
||||
|
||||
let mut end = start;
|
||||
|
||||
while let Some((idx, p)) =
|
||||
dynamic_nodes.next_if(|(_, p)| matches!(p, [idx, ..] if *idx == root_idx))
|
||||
{
|
||||
if p.len() == 1 {
|
||||
continue;
|
||||
}
|
||||
|
||||
end = idx;
|
||||
}
|
||||
|
||||
Some((start, end))
|
||||
}
|
||||
|
||||
let (start, end) = match collect_dyn_node_range(dynamic_nodes_iter, root_idx) {
|
||||
Some((a, b)) => (a, b),
|
||||
None => return,
|
||||
};
|
||||
|
||||
// !!VERY IMPORTANT!!
|
||||
//
|
||||
// We need to walk the dynamic nodes in reverse order because we are going to replace the
|
||||
// placeholder with the new nodes, which will invalidate our paths into the template.
|
||||
// If we go in reverse, we leave a "wake of destruction" in our path, but our next iteration
|
||||
// will still be "clean" since we only invalidated downstream nodes.
|
||||
//
|
||||
// Forgetting to do this will cause weird bugs like:
|
||||
// https://github.com/DioxusLabs/dioxus/issues/2809
|
||||
//
|
||||
// Which are quite serious.
|
||||
// There might be more places in this codebase where we need to do `.rev()`
|
||||
let reversed_iter = (start..=end).rev();
|
||||
|
||||
for dynamic_node_id in reversed_iter {
|
||||
let m = self.create_dynamic_node(
|
||||
&self.dynamic_nodes[dynamic_node_id],
|
||||
mount,
|
||||
@@ -708,7 +749,7 @@ impl VNode {
|
||||
// If we actually created real new nodes, we need to replace the placeholder for this dynamic node with the new dynamic nodes
|
||||
if m > 0 {
|
||||
// The path is one shorter because the top node is the root
|
||||
let path = &path[1..];
|
||||
let path = &self.template.node_paths[dynamic_node_id][1..];
|
||||
to.replace_placeholder_with_nodes(path, m);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
#![allow(non_snake_case)]
|
||||
|
||||
use dioxus::dioxus_core::Mutation::*;
|
||||
use dioxus::prelude::*;
|
||||
use dioxus_core::ElementId;
|
||||
|
||||
/// Should push the text node onto the stack and modify it
|
||||
#[test]
|
||||
fn many_roots() {
|
||||
fn app() -> Element {
|
||||
rsx! {
|
||||
div {
|
||||
MyNav {}
|
||||
MyOutlet {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn MyNav() -> Element {
|
||||
rsx!(
|
||||
div { "trailing nav" }
|
||||
div { "whhhhh"}
|
||||
div { "bhhhh" }
|
||||
)
|
||||
}
|
||||
|
||||
fn MyOutlet() -> Element {
|
||||
rsx!(
|
||||
div { "homepage 1" }
|
||||
)
|
||||
}
|
||||
|
||||
let mut dom = VirtualDom::new(app);
|
||||
let edits = dom.rebuild_to_vec();
|
||||
|
||||
assert_eq!(
|
||||
edits.edits,
|
||||
[
|
||||
// load the div {} container
|
||||
LoadTemplate { index: 0, id: ElementId(1) },
|
||||
// Load myoutlet first
|
||||
LoadTemplate { index: 0, id: ElementId(2) },
|
||||
ReplacePlaceholder { path: &[1], m: 1 },
|
||||
// Then the MyNav
|
||||
LoadTemplate { index: 0, id: ElementId(3) },
|
||||
LoadTemplate { index: 1, id: ElementId(4) },
|
||||
LoadTemplate { index: 2, id: ElementId(5) },
|
||||
ReplacePlaceholder { path: &[0], m: 3 },
|
||||
// Then mount the div to the dom
|
||||
AppendChildren { m: 1, id: ElementId(0) },
|
||||
]
|
||||
)
|
||||
}
|
||||
@@ -97,14 +97,17 @@ impl WriteMutations for MutationState {
|
||||
fn load_template(&mut self, template: Template, index: usize, id: dioxus_core::ElementId) {
|
||||
// Get the template or create it if we haven't seen it before
|
||||
let tmpl_id = self.templates.get(&template).cloned().unwrap_or_else(|| {
|
||||
let current_max_template_count = self.templates.len() as u16;
|
||||
let tmpl_id = self.templates.len() as u16;
|
||||
self.templates.insert(template, tmpl_id);
|
||||
|
||||
for root in template.roots.iter() {
|
||||
self.create_template_node(root);
|
||||
self.templates.insert(template, current_max_template_count);
|
||||
}
|
||||
let id = template.roots.len() as u16;
|
||||
self.channel.add_templates(current_max_template_count, id);
|
||||
current_max_template_count
|
||||
|
||||
let len = template.roots.len() as u16;
|
||||
self.channel.add_templates(tmpl_id, len);
|
||||
|
||||
tmpl_id
|
||||
});
|
||||
|
||||
self.channel
|
||||
|
||||
Reference in new issue
Block a user