diff --git a/packages/core/src/diff/node.rs b/packages/core/src/diff/node.rs index e077f633f..69fb1114b 100644 --- a/packages/core/src/diff/node.rs +++ b/packages/core/src/diff/node.rs @@ -592,6 +592,12 @@ impl VNode { // If this is an element, load in all of the placeholder or dynamic content under this root element too if matches!(root, TemplateNode::Element { .. }) { + // !!VERY IMPORTANT!! + // Write out all attributes before we load the children. Loading the children will change paths we rely on + // to assign ids to elements with dynamic attributes + if let Some(to) = to.as_deref_mut() { + self.write_attrs(mount, &mut attrs, root_idx as u8, dom, to); + } // This operation relies on the fact that the root node is the top node on the stack so we need to do it here self.load_placeholders( mount, @@ -600,10 +606,6 @@ impl VNode { dom, to.as_deref_mut(), ); - // Now write out any attributes we need - if let Some(to) = to.as_deref_mut() { - self.write_attrs(mount, &mut attrs, root_idx as u8, dom, to); - } } // This creates one node on the stack diff --git a/packages/core/tests/kitchen_sink.rs b/packages/core/tests/kitchen_sink.rs index 7923e29d1..aa61612e8 100644 --- a/packages/core/tests/kitchen_sink.rs +++ b/packages/core/tests/kitchen_sink.rs @@ -1,5 +1,6 @@ use dioxus::dioxus_core::{ElementId, Mutation}; use dioxus::prelude::*; +use pretty_assertions::assert_eq; fn basic_syntax_is_a_template() -> Element { let asd = 123; @@ -39,8 +40,6 @@ fn dual_stream() { assert_eq!(edits.edits, { [ LoadTemplate { index: 0, id: ElementId(1) }, - CreateTextNode { value: "123".to_string(), id: ElementId(2) }, - ReplacePlaceholder { path: &[0, 0], m: 1 }, SetAttribute { name: "class", value: "asd 123 123 ".into_value(), @@ -48,6 +47,8 @@ fn dual_stream() { ns: None, }, NewEventListener { name: "click".to_string(), id: ElementId(1) }, + CreateTextNode { value: "123".to_string(), id: ElementId(2) }, + ReplacePlaceholder { path: &[0, 0], m: 1 }, AppendChildren { id: ElementId(0), m: 1 }, ] }); diff --git a/packages/core/tests/many_roots.rs b/packages/core/tests/many_roots.rs index 48ffe7fd2..c954df52b 100644 --- a/packages/core/tests/many_roots.rs +++ b/packages/core/tests/many_roots.rs @@ -2,16 +2,24 @@ use dioxus::dioxus_core::Mutation::*; use dioxus::prelude::*; -use dioxus_core::ElementId; +use dioxus_core::{AttributeValue, ElementId}; +use pretty_assertions::assert_eq; /// Should push the text node onto the stack and modify it +/// Regression test for https://github.com/DioxusLabs/dioxus/issues/2809 and https://github.com/DioxusLabs/dioxus/issues/3055 #[test] fn many_roots() { fn app() -> Element { + let width = "100%"; rsx! { div { MyNav {} MyOutlet {} + div { + // We need to make sure that dynamic attributes are set before the nodes before them are expanded + // If they are set after, then the paths are incorrect + width, + } } } } @@ -38,13 +46,21 @@ fn many_roots() { [ // 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 + // Set the width attribute first + AssignId { path: &[2], id: ElementId(2,) }, + SetAttribute { + name: "width", + ns: Some("style",), + value: AttributeValue::Text("100%".to_string()), + id: ElementId(2,), + }, + // Load MyOutlet next LoadTemplate { index: 0, id: ElementId(3) }, - LoadTemplate { index: 1, id: ElementId(4) }, - LoadTemplate { index: 2, id: ElementId(5) }, + ReplacePlaceholder { path: &[1], m: 1 }, + // Then MyNav + LoadTemplate { index: 0, id: ElementId(4) }, + LoadTemplate { index: 1, id: ElementId(5) }, + LoadTemplate { index: 2, id: ElementId(6) }, ReplacePlaceholder { path: &[0], m: 3 }, // Then mount the div to the dom AppendChildren { m: 1, id: ElementId(0) }, diff --git a/packages/core/tests/suspense.rs b/packages/core/tests/suspense.rs index cfabe841f..9beb39655 100644 --- a/packages/core/tests/suspense.rs +++ b/packages/core/tests/suspense.rs @@ -623,37 +623,37 @@ fn nested_suspense_resolves_client() { // Load the title LoadTemplate { index: 0, id: ElementId(2,) }, - CreateTextNode { value: "The robot says hello world".to_string(), id: ElementId(4,) }, - ReplacePlaceholder { path: &[0,], m: 1 }, SetAttribute { name: "id", ns: None, value: AttributeValue::Text("title-0".to_string()), id: ElementId(2,), }, + CreateTextNode { value: "The robot says hello world".to_string(), id: ElementId(4,) }, + ReplacePlaceholder { path: &[0,], m: 1 }, // Then load the body LoadTemplate { index: 1, id: ElementId(5,) }, - CreateTextNode { value: "The robot becomes sentient and says hello world".to_string(), id: ElementId(6,) }, - ReplacePlaceholder { path: &[0,], m: 1 }, SetAttribute { name: "id", ns: None, value: AttributeValue::Text("body-0".to_string()), id: ElementId(5,), }, + CreateTextNode { value: "The robot becomes sentient and says hello world".to_string(), id: ElementId(6,) }, + ReplacePlaceholder { path: &[0,], m: 1 }, // Then load the suspended children LoadTemplate { index: 2, id: ElementId(7,) }, - CreateTextNode { value: "Loading 1...".to_string(), id: ElementId(8,) }, - CreateTextNode { value: "Loading 2...".to_string(), id: ElementId(9,) }, - ReplacePlaceholder { path: &[0,], m: 2 }, SetAttribute { name: "id", ns: None, value: AttributeValue::Text("children-0".to_string()), id: ElementId(7,), }, + CreateTextNode { value: "Loading 1...".to_string(), id: ElementId(8,) }, + CreateTextNode { value: "Loading 2...".to_string(), id: ElementId(9,) }, + ReplacePlaceholder { path: &[0,], m: 2 }, // Finally replace the loading placeholder in the body with the resolved children ReplaceWith { id: ElementId(3,), m: 3 }, @@ -693,13 +693,6 @@ fn nested_suspense_resolves_client() { 8, ), }, - CreateTextNode { value: "The world says hello back".to_string(), id: ElementId(10,) }, - ReplacePlaceholder { - path: &[ - 0, - ], - m: 1, - }, SetAttribute { name: "id", ns: None, @@ -708,13 +701,27 @@ fn nested_suspense_resolves_client() { 8, ), }, + CreateTextNode { value: "The world says hello back".to_string(), id: ElementId(10,) }, + ReplacePlaceholder { + path: &[ + 0, + ], + m: 1, + }, LoadTemplate { - index: 1, id: ElementId( 11, ), }, + SetAttribute { + name: "id", + ns: None, + value: AttributeValue::Text("body-1".to_string()), + id: ElementId( + 11, + ), + }, CreateTextNode { value: "In a stunning turn of events, the world collectively unites and says hello back".to_string(), id: ElementId(12,) }, ReplacePlaceholder { path: &[ @@ -722,16 +729,16 @@ fn nested_suspense_resolves_client() { ], m: 1, }, + LoadTemplate { + index: 2, + id: ElementId( + 13, + ), + }, SetAttribute { name: "id", ns: None, - value: AttributeValue::Text("body-1".to_string()), - id: ElementId( - 11, - ), - }, - LoadTemplate { - index: 2, + value: AttributeValue::Text("children-1".to_string()), id: ElementId( 13, ), @@ -743,14 +750,6 @@ fn nested_suspense_resolves_client() { ], m: 1, }, - SetAttribute { - name: "id", - ns: None, - value: AttributeValue::Text("children-1".to_string()), - id: ElementId( - 13, - ), - }, ReplaceWith { id: ElementId( 3, @@ -776,13 +775,6 @@ fn nested_suspense_resolves_client() { 9, ), }, - CreateTextNode { value: "Goodbye Robot".to_string(), id: ElementId(15,) }, - ReplacePlaceholder { - path: &[ - 0, - ], - m: 1, - }, SetAttribute { name: "id", ns: None, @@ -791,12 +783,27 @@ fn nested_suspense_resolves_client() { 9, ), }, + CreateTextNode { value: "Goodbye Robot".to_string(), id: ElementId(15,) }, + ReplacePlaceholder { + path: &[ + 0, + ], + m: 1, + }, LoadTemplate { index: 1, id: ElementId( 16, ), }, + SetAttribute { + name: "id", + ns: None, + value: AttributeValue::Text("body-2".to_string()), + id: ElementId( + 16, + ), + }, CreateTextNode { value: "The robot says goodbye".to_string(), id: ElementId(17,) }, ReplacePlaceholder { path: &[ @@ -804,14 +811,6 @@ fn nested_suspense_resolves_client() { ], m: 1, }, - SetAttribute { - name: "id", - ns: None, - value: AttributeValue::Text("body-2".to_string()), - id: ElementId( - 16, - ), - }, LoadTemplate { index: 2, @@ -819,9 +818,6 @@ fn nested_suspense_resolves_client() { 18, ), }, - // Create a placeholder for the resolved children - CreateTextNode { value: "Loading 3...".to_string(), id: ElementId(19,) }, - ReplacePlaceholder { path: &[0,], m: 1 }, SetAttribute { name: "id", ns: None, @@ -830,6 +826,9 @@ fn nested_suspense_resolves_client() { 18, ), }, + // Create a placeholder for the resolved children + CreateTextNode { value: "Loading 3...".to_string(), id: ElementId(19,) }, + ReplacePlaceholder { path: &[0,], m: 1 }, // Replace the loading placeholder with the resolved children ReplaceWith { @@ -864,13 +863,6 @@ fn nested_suspense_resolves_client() { 19, ), }, - CreateTextNode { value: "Goodbye Robot again".to_string(), id: ElementId(20,) }, - ReplacePlaceholder { - path: &[ - 0, - ], - m: 1, - }, SetAttribute { name: "id", ns: None, @@ -879,12 +871,27 @@ fn nested_suspense_resolves_client() { 19, ), }, + CreateTextNode { value: "Goodbye Robot again".to_string(), id: ElementId(20,) }, + ReplacePlaceholder { + path: &[ + 0, + ], + m: 1, + }, LoadTemplate { index: 1, id: ElementId( 21, ), }, + SetAttribute { + name: "id", + ns: None, + value: AttributeValue::Text("body-3".to_string()), + id: ElementId( + 21, + ), + }, CreateTextNode { value: "The robot says goodbye again".to_string(), id: ElementId(22,) }, ReplacePlaceholder { path: &[ @@ -892,16 +899,16 @@ fn nested_suspense_resolves_client() { ], m: 1, }, + LoadTemplate { + index: 2, + id: ElementId( + 23, + ), + }, SetAttribute { name: "id", ns: None, - value: AttributeValue::Text("body-3".to_string()), - id: ElementId( - 21, - ), - }, - LoadTemplate { - index: 2, + value: AttributeValue::Text("children-3".to_string()), id: ElementId( 23, ), @@ -913,14 +920,6 @@ fn nested_suspense_resolves_client() { ], m: 1, }, - SetAttribute { - name: "id", - ns: None, - value: AttributeValue::Text("children-3".to_string()), - id: ElementId( - 23, - ), - }, ReplaceWith { id: ElementId( 3,