Parcourir la source

Fix inserting attributes after expanding dynamic nodes (#3094)

Evan Almloff il y a 8 mois
Parent
commit
ef436e4ed0

+ 6 - 4
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

+ 3 - 2
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 },
         ]
     });

+ 23 - 7
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) },

+ 66 - 67
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,20 +701,19 @@ fn nested_suspense_resolves_client() {
                             8,
                         ),
                     },
-                    LoadTemplate {
-
-                        index: 1,
-                        id: ElementId(
-                            11,
-                        ),
-                    },
-                    CreateTextNode { value: "In a stunning turn of events, the world collectively unites and says hello back".to_string(), id: ElementId(12,) },
+                    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,
@@ -730,19 +722,19 @@ fn nested_suspense_resolves_client() {
                             11,
                         ),
                     },
-                    LoadTemplate {
-                        index: 2,
-                        id: ElementId(
-                            13,
-                        ),
-                    },
-                    CreatePlaceholder { id: ElementId(14,) },
+                    CreateTextNode { value: "In a stunning turn of events, the world collectively unites and says hello back".to_string(), id: ElementId(12,) },
                     ReplacePlaceholder {
                         path: &[
                             0,
                         ],
                         m: 1,
                     },
+                    LoadTemplate {
+                        index: 2,
+                        id: ElementId(
+                            13,
+                        ),
+                    },
                     SetAttribute {
                         name: "id",
                         ns: None,
@@ -751,6 +743,13 @@ fn nested_suspense_resolves_client() {
                             13,
                         ),
                     },
+                    CreatePlaceholder { id: ElementId(14,) },
+                    ReplacePlaceholder {
+                        path: &[
+                            0,
+                        ],
+                        m: 1,
+                    },
                     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,19 +783,19 @@ fn nested_suspense_resolves_client() {
                             9,
                         ),
                     },
-                    LoadTemplate {
-                        index: 1,
-                        id: ElementId(
-                            16,
-                        ),
-                    },
-                    CreateTextNode { value: "The robot says goodbye".to_string(), id: ElementId(17,) },
+                    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,
@@ -812,6 +804,13 @@ fn nested_suspense_resolves_client() {
                             16,
                         ),
                     },
+                    CreateTextNode { value: "The robot says goodbye".to_string(), id: ElementId(17,) },
+                    ReplacePlaceholder {
+                        path: &[
+                            0,
+                        ],
+                        m: 1,
+                    },
                     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,19 +871,19 @@ fn nested_suspense_resolves_client() {
                             19,
                         ),
                     },
-                    LoadTemplate {
-                        index: 1,
-                        id: ElementId(
-                            21,
-                        ),
-                    },
-                    CreateTextNode { value: "The robot says goodbye again".to_string(), id: ElementId(22,) },
+                    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,
@@ -900,19 +892,19 @@ fn nested_suspense_resolves_client() {
                             21,
                         ),
                     },
+                    CreateTextNode { value: "The robot says goodbye again".to_string(), id: ElementId(22,) },
+                    ReplacePlaceholder {
+                        path: &[
+                            0,
+                        ],
+                        m: 1,
+                    },
                     LoadTemplate {
                         index: 2,
                         id: ElementId(
                             23,
                         ),
                     },
-                    CreatePlaceholder { id: ElementId(24,) },
-                    ReplacePlaceholder {
-                        path: &[
-                            0
-                        ],
-                        m: 1,
-                    },
                     SetAttribute {
                         name: "id",
                         ns: None,
@@ -921,6 +913,13 @@ fn nested_suspense_resolves_client() {
                             23,
                         ),
                     },
+                    CreatePlaceholder { id: ElementId(24,) },
+                    ReplacePlaceholder {
+                        path: &[
+                            0
+                        ],
+                        m: 1,
+                    },
                     ReplaceWith {
                         id: ElementId(
                             3,