Properly handle dynamic nodes in a fragment (sycamore-rs#251)

simonchatts · Sep 18, 2021 · c8909c3 · c8909c3
1 parent 97b38a4
commit c8909c3
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 3 deletions.
diff --git a/packages/sycamore/src/utils/render.rs b/packages/sycamore/src/utils/render.rs
@@ -78,12 +78,16 @@ fn insert_expression<G: GenericNode>(
         }
         TemplateType::Fragment(fragment) => {
             let mut v = Vec::new();
+            // normalize_incoming_fragment will subscribe to all dynamic nodes in the function so as
+            // to trigger the create_effect when the template changes.
             let dynamic = normalize_incoming_fragment(&mut v, fragment.as_ref(), unwrap_fragment);
             if dynamic {
                 let parent = parent.clone();
                 let marker = marker.cloned();
                 create_effect(move || {
                     let value = Template::new_fragment(v.clone());
+                    // This will call normalize_incoming_fragment again, but this time with the
+                    // unwrap_fragment arg set to true.
                     insert_expression(
                         &parent,
                         &value,
@@ -92,9 +96,13 @@ fn insert_expression<G: GenericNode>(
                         true,
                         false,
                     );
-                    current = Some(value); // FIXME: should be return value of
-                                           // normalize_incoming_fragment called in recursive
-                                           // insert_expression
+                    current = Some(Template::new_fragment(
+                        value
+                            .flatten()
+                            .into_iter()
+                            .map(Template::new_node)
+                            .collect(),
+                    )); // TODO: do not perform unnecessary flattening of template
                 });
             } else {
                 let v = v

diff --git a/packages/sycamore/tests/web/main.rs b/packages/sycamore/tests/web/main.rs
@@ -190,6 +190,53 @@ fn template_interpolation_if_else() {
     );
 }
 
+#[wasm_bindgen_test]
+fn template_interpolation_if_else_with_sibling() {
+    let show = Signal::new(true);
+    let node = cloned!((show) => template! {
+        div { "Before" }
+        (if *show.get() {
+            template! { p { "Hello Sycamore!" } }
+        } else {
+            template! { p { "" }}
+        })
+    });
+
+    sycamore::render_to(|| node, &test_container());
+
+    assert_eq!(
+        document()
+            .query_selector("p")
+            .unwrap()
+            .unwrap()
+            .text_content()
+            .unwrap(),
+        "Hello Sycamore!"
+    );
+
+    show.set(false);
+    assert_eq!(
+        document()
+            .query_selector("p")
+            .unwrap()
+            .unwrap()
+            .text_content()
+            .unwrap(),
+        ""
+    );
+
+    show.set(true);
+    assert_eq!(
+        document()
+            .query_selector("p")
+            .unwrap()
+            .unwrap()
+            .text_content()
+            .unwrap(),
+        "Hello Sycamore!"
+    );
+}
+
 #[wasm_bindgen_test]
 fn template_interpolation_nested_reactivity() {
     let count = Signal::new(0);