fix(core): 🐛 the class update context is not clean and may result in …

…missing mounted widgets

CHANGELOG.md

@@ -31,7 +31,8 @@ Please only add new entries below the [Unreleased](#unreleased---releasedate) he
 
 ### Fixed
 
-- **core**: The `Provider` might be missing in a pipe class. (#pr @M-Adoo)
+- **core**: The `Provider` might be missing in a pipe class. (#648 @M-Adoo)
+- **core**: The child generated by the class may not be mounted. (#648 @M-Adoo)
 
 ## [0.4.0-alpha.14] - 2024-10-30
 

core/src/builtin_widgets/class.rs

@@ -208,21 +208,29 @@ impl ClassChild {
 
   fn update(&self, orig: &OrigChild, class: &Class, ctx: &mut BuildCtx) {
     let InnerClassChild { child, child_id, orig_id } = self.inner();
+    eprintln!("old tree\n {}", ctx.tree().display_tree(ctx.tree().root()));
+    let cls_holder = child_id.place_holder(ctx.tree());
 
-    // Set the widget generated by the class back to the tree node. However, retain
-    // the tree node for reuse with the new ID as it may contain attached
-    // information or be combined with other render widgets.
-    let class_child = self.take_inner();
-    let class_node = std::mem::replace(child_id.get_node_mut(ctx.tree_mut()).unwrap(), class_child);
-
-    // Revert to the original child.
-    *orig_id.get_node_mut(ctx.tree_mut()).unwrap() = Box::new(orig.clone());
-
+    // Extract the child from this node, retaining only the external information
+    // linked from the parent to create a clean context for applying the class.
+    let child_node = self.take_inner();
+    let n_orig = ctx.alloc(Box::new(orig.clone()));
     let new_id = class
-      .apply_style(Widget::from_id(*orig_id), ctx)
+      .apply_style(Widget::from_id(n_orig), ctx)
       .build(ctx);
-
     let tree = ctx.tree_mut();
+    // Place the inner child node within the old ID for disposal, then utilize the
+    // class node to wrap the new child in the new ID.
+    // This action should be taken before modifying the `orig_id`, as the `orig_id`
+    // may be the same as the `child_id`.
+    let class_node = std::mem::replace(child_id.get_node_mut(tree).unwrap(), child_node);
+
+    // Retain the original widget ID.
+    let [new, old] = tree.get_many_mut(&[n_orig, *orig_id]);
+    std::mem::swap(new, old);
+    n_orig.insert_after(*orig_id, tree);
+    n_orig.dispose_subtree(tree);
+
     new_id.wrap_node(tree, |node| {
       *child = node;
       class_node
@@ -242,10 +250,10 @@ impl ClassChild {
             .borrow_mut()
             .single_range_replace(&old_rg, &new_rg)
         });
+      cls_holder.replace(new_id, tree);
     }
 
     if orig_id != child_id {
-      child_id.insert_after(new_id, tree);
       child_id.dispose_subtree(tree);
     }
 
@@ -259,8 +267,9 @@ impl ClassChild {
     }
 
     *child_id = new_id;
-
     tree.mark_dirty(new_id);
+    tree.mark_dirty(*orig_id);
+    eprintln!("new tree\n {}", ctx.tree().display_tree(ctx.tree().root()));
   }
 
   #[allow(clippy::mut_from_ref)]
@@ -337,18 +346,12 @@ mod tests {
     reset_test_env,
     test_helper::{MockBox, MockMulti, TestWindow, split_value},
   };
+  class_names!(MARGIN, BOX_200, EMPTY);
 
-  #[test]
-  fn switch_class() {
-    reset_test_env!();
-
-    class_names!(MARGIN, SCALE_2X);
-
-    let mut theme = Theme::default();
-    theme
-      .classes
-      .insert(MARGIN, style_class!(margin: EdgeInsets::all(10.)));
-    theme.classes.insert(SCALE_2X, |w| {
+  fn initd_classes() -> Classes {
+    let mut classes = Classes::default();
+    classes.insert(MARGIN, style_class!(margin: EdgeInsets::all(10.)));
+    classes.insert(BOX_200, |w| {
       fn_widget! {
         @MockBox {
           size: Size::new(200., 200.),
@@ -357,21 +360,28 @@ mod tests {
       }
       .into_widget()
     });
-    AppCtx::set_app_theme(theme);
+    classes
+  }
 
-    let (cls, w_cls) = split_value(MARGIN);
+  #[test]
+  fn switch_class() {
+    reset_test_env!();
 
+    let (cls, w_cls) = split_value(MARGIN);
     let mut wnd = TestWindow::new(fn_widget! {
-      @Container {
-        size: Size::new(100., 100.),
-        class: pipe!(*$cls),
-      }
+      let cls = cls.clone_watcher();
+      initd_classes().with_child(fn_widget! {
+        @Container {
+          size: Size::new(100., 100.),
+          class: pipe!(*$cls),
+        }
+      })
     });
 
     wnd.draw_frame();
     wnd.assert_root_size(Size::new(120., 120.));
 
-    *w_cls.write() = SCALE_2X;
+    *w_cls.write() = BOX_200;
     wnd.draw_frame();
     wnd.assert_root_size(Size::new(200., 200.));
   }
@@ -381,31 +391,29 @@ mod tests {
   fn on_disposed_of_class_nodes() {
     reset_test_env!();
 
-    class_names!(ON_DISPOSED, MARGIN);
-
-    let mut theme = Theme::default();
-    theme.classes.insert(MARGIN, style_class! {
-      margin: EdgeInsets::all(10.)
-    });
-    theme.classes.insert(ON_DISPOSED, |w| {
-      fn_widget! {
-        @MockBox {
-          size: Size::zero(),
-          on_disposed: move |_| panic!("on_disposed called"),
-          @ { w }
-        }
-      }
-      .into_widget()
-    });
-    AppCtx::set_app_theme(theme);
+    class_names!(ON_DISPOSED);
 
     let (cls, w_cls) = split_value(ON_DISPOSED);
 
     let mut wnd = TestWindow::new(fn_widget! {
-      @Container {
-        size: Size::new(100., 100.),
-        class: pipe!(*$cls),
-      }
+      let cls = cls.clone_watcher();
+      let mut classes = initd_classes();
+      classes.insert(ON_DISPOSED, |w| {
+        fn_widget! {
+          @MockBox {
+            size: Size::zero(),
+            on_disposed: move |_| panic!("on_disposed called"),
+            @ { w }
+          }
+        }
+        .into_widget()
+      });
+      classes.with_child(fn_widget! {
+        @Container {
+          size: Size::new(100., 100.),
+          class: pipe!(*$cls),
+        }
+      })
     });
 
     wnd.draw_frame();
@@ -417,32 +425,29 @@ mod tests {
   fn fix_crash_for_class_impl_may_have_multi_child() {
     reset_test_env!();
 
-    class_names!(MULTI, SINGLE);
-    let mut theme = Theme::default();
-    theme
-      .classes
-      .insert(SINGLE, style_class! { margin: EdgeInsets::all(10.) });
-    theme.classes.insert(MULTI, |w| {
-      // A class implementation may consist of a complex subtree, and it cannot be
-      // guaranteed to have a single linked structure.
-      fn_widget! {
-        @MockMulti {
-          @MockBox { size: Size::new(100., 100.) }
-          @MockBox { size: Size::new(100., 200.) }
-          @ { w }
-        }
-      }
-      .into_widget()
-    });
-    AppCtx::set_app_theme(theme);
+    class_names!(MULTI);
 
-    let (cls, w_cls) = split_value(SINGLE);
+    let (cls, w_cls) = split_value(MARGIN);
 
     let mut wnd = TestWindow::new(fn_widget! {
-      @Container {
-        size: Size::new(100., 100.),
-        class: pipe!(*$cls),
-      }
+      let cls = cls.clone_watcher();
+      let mut classes = initd_classes();
+      classes.insert(MULTI, |w| {
+        fn_widget! {
+          @MockMulti {
+            @MockBox { size: Size::new(100., 100.) }
+            @MockBox { size: Size::new(100., 200.) }
+            @ { w }
+          }
+        }
+        .into_widget()
+      });
+      classes.with_child(fn_widget! {
+        @Container {
+          size: Size::new(100., 100.),
+          class: pipe!(*$cls),
+        }
+      })
     });
 
     wnd.draw_frame();
@@ -480,4 +485,27 @@ mod tests {
     });
     wnd.draw_frame();
   }
+
+  #[test]
+  fn fix_not_mounted_class_node() {
+    reset_test_env!();
+
+    let (cls, w_cls) = split_value(EMPTY);
+    let mut wnd = TestWindow::new(fn_widget! {
+      let cls = cls.clone_watcher();
+      initd_classes().with_child(fn_widget! {
+        @Container {
+          size: Size::new(100., 100.),
+          class: pipe!(*$cls),
+        }
+      })
+    });
+
+    wnd.draw_frame();
+    wnd.assert_root_size(Size::new(100., 100.));
+
+    *w_cls.write() = BOX_200;
+    wnd.draw_frame();
+    wnd.assert_root_size(Size::new(200., 200.));
+  }
 }

core/src/widget_tree/widget_id.rs

@@ -14,6 +14,13 @@ use crate::{
 
 pub struct WidgetId(pub(crate) NodeId);
 
+// A place holder get from a `WidgetId`, you can use it to insert a widget
+// replace the `WidgetId` you get this place holder.
+pub(crate) enum PlaceHolder {
+  PrevSibling(WidgetId),
+  Parent(WidgetId),
+}
+
 pub trait RenderQueryable: Render + Query {
   fn as_render(&self) -> &dyn Render;
 }
@@ -75,6 +82,14 @@ impl WidgetId {
       .map(|(a, _)| a)
   }
 
+  pub(crate) fn place_holder(self, tree: &WidgetTree) -> PlaceHolder {
+    if let Some(prev) = self.prev_sibling(tree) {
+      PlaceHolder::PrevSibling(prev)
+    } else {
+      PlaceHolder::Parent(self.parent(tree).unwrap())
+    }
+  }
+
   pub(crate) fn parent(self, tree: &WidgetTree) -> Option<WidgetId> {
     self.node_feature(tree, |node| node.parent())
   }
@@ -299,3 +314,12 @@ impl WidgetId {
 
   pub(crate) fn queryable(&self, tree: &WidgetTree) -> bool { self.assert_get(tree).queryable() }
 }
+
+impl PlaceHolder {
+  pub(crate) fn replace(self, widget: WidgetId, tree: &mut WidgetTree) {
+    match self {
+      PlaceHolder::PrevSibling(prev) => prev.insert_after(widget, tree),
+      PlaceHolder::Parent(parent) => parent.append(widget, tree),
+    }
+  }
+}