Partially undo #2673, different approach for the DerefMut impl of VList (#2692)

* partially undo #2673

VList again has a DerefMut implementation
the internal fully_keyed state now has an "indeterminate" variant
instead of being a bool, this recomputes it during reconciliation

* add Copy impl to FullyKeyedState

examples/futures/src/markdown.rs

@@ -54,12 +54,10 @@ pub fn render_markdown(src: &str) -> Html {
                     top = pre;
                 } else if let Tag::Table(aligns) = tag {
                     if let Some(top_children) = top.children_mut() {
-                        for r in top_children.children_mut().iter_mut() {
+                        for r in top_children.iter_mut() {
                             if let VNode::VTag(ref mut vtag) = r {
                                 if let Some(vtag_children) = vtag.children_mut() {
-                                    for (i, c) in
+                                    for (i, c) in vtag_children.iter_mut().enumerate() {
-                                        vtag_children.children_mut().iter_mut().enumerate()
-                                    {
                                         if let VNode::VTag(ref mut vtag) = c {
                                             match aligns[i] {
                                                 Alignment::None => {}
@@ -75,7 +73,7 @@ pub fn render_markdown(src: &str) -> Html {
                     }
                 } else if let Tag::TableHead = tag {
                     if let Some(top_children) = top.children_mut() {
-                        for c in top_children.children_mut().iter_mut() {
+                        for c in top_children.iter_mut() {
                             if let VNode::VTag(ref mut vtag) = c {
                                 // TODO
                                 //                            vtag.tag = "th".into();

packages/yew/src/dom_bundle/blist.rs

@@ -444,6 +444,7 @@ impl Reconcilable for VList {
             self.add_child(VText::new("").into());
         }
 
+        let fully_keyed = self.fully_keyed();
         let lefts = self.children;
         let rights = &mut blist.rev_children;
         test_log!("lefts: {:?}", lefts);
@@ -452,12 +453,12 @@ impl Reconcilable for VList {
         if let Some(additional) = lefts.len().checked_sub(rights.len()) {
             rights.reserve_exact(additional);
         }
-        let first = if self.fully_keyed && blist.fully_keyed {
+        let first = if fully_keyed && blist.fully_keyed {
             BList::apply_keyed(root, parent_scope, parent, next_sibling, lefts, rights)
         } else {
             BList::apply_unkeyed(root, parent_scope, parent, next_sibling, lefts, rights)
         };
-        blist.fully_keyed = self.fully_keyed;
+        blist.fully_keyed = fully_keyed;
         blist.key = self.key;
         test_log!("result: {:?}", rights);
         first
@@ -478,9 +479,11 @@ mod feat_hydration {
             fragment: &mut Fragment,
         ) -> (NodeRef, Self::Bundle) {
             let node_ref = NodeRef::default();
-            let mut children = Vec::with_capacity(self.children.len());
+            let fully_keyed = self.fully_keyed();
+            let vchildren = self.children;
+            let mut children = Vec::with_capacity(vchildren.len());
 
-            for (index, child) in self.children.into_iter().enumerate() {
+            for (index, child) in vchildren.into_iter().enumerate() {
                 let (child_node_ref, child) = child.hydrate(root, parent_scope, parent, fragment);
 
                 if index == 0 {
@@ -496,7 +499,7 @@ mod feat_hydration {
                 node_ref,
                 BList {
                     rev_children: children,
-                    fully_keyed: self.fully_keyed,
+                    fully_keyed,
                     key: self.key,
                 },
             )

packages/yew/src/virtual_dom/vlist.rs

@@ -3,18 +3,31 @@ use std::ops::{Deref, DerefMut};
 
 use super::{Key, VNode};
 
+#[derive(Clone, Copy, Debug, PartialEq)]
+enum FullyKeyedState {
+    KnownFullyKeyed,
+    KnownMissingKeys,
+    Unknown,
+}
+
 /// This struct represents a fragment of the Virtual DOM tree.
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug)]
 pub struct VList {
     /// The list of child [VNode]s
     pub(crate) children: Vec<VNode>,
 
     /// All [VNode]s in the VList have keys
-    pub(crate) fully_keyed: bool,
+    fully_keyed: FullyKeyedState,
 
     pub key: Option<Key>,
 }
 
+impl PartialEq for VList {
+    fn eq(&self, other: &Self) -> bool {
+        self.children == other.children && self.key == other.key
+    }
+}
+
 impl Default for VList {
     fn default() -> Self {
         Self::new()
@@ -29,42 +42,10 @@ impl Deref for VList {
     }
 }
 
-/// Mutable children of a [VList].
+impl DerefMut for VList {
-///
-/// This struct has a `DerefMut` implementations into [`Vec<VNode>`](std::vec::Vec).
-/// Prefer to use immutable access, since this re-checks if all nodes have keys when dropped.
-pub struct ChildrenMut<'a> {
-    children: &'a mut Vec<VNode>,
-    fully_keyed: &'a mut bool,
-}
-
-impl<'a> Deref for ChildrenMut<'a> {
-    type Target = Vec<VNode>;
-
-    fn deref(&self) -> &Self::Target {
-        self.children
-    }
-}
-
-impl<'a> DerefMut for ChildrenMut<'a> {
     fn deref_mut(&mut self) -> &mut Self::Target {
-        *self.fully_keyed = false;
+        self.fully_keyed = FullyKeyedState::Unknown;
-        self.children
+        &mut self.children
-    }
-}
-
-impl<'a> Drop for ChildrenMut<'a> {
-    fn drop(&mut self) {
-        if !*self.fully_keyed {
-            // Caller might have changed the keys of the VList or add unkeyed children.
-            *self.fully_keyed = self.children.iter().all(|ch| ch.has_key());
-        }
-    }
-}
-
-impl<'a> std::fmt::Debug for ChildrenMut<'a> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_tuple("ChildrenMut").field(&self.children).finish()
     }
 }
 
@@ -74,23 +55,29 @@ impl VList {
         Self {
             children: Vec::new(),
             key: None,
-            fully_keyed: true,
+            fully_keyed: FullyKeyedState::KnownFullyKeyed,
         }
     }
 
     /// Creates a new [VList] instance with children.
     pub fn with_children(children: Vec<VNode>, key: Option<Key>) -> Self {
-        VList {
+        let mut vlist = VList {
-            fully_keyed: children.iter().all(|ch| ch.has_key()),
+            fully_keyed: FullyKeyedState::Unknown,
             children,
             key,
-        }
+        };
+        vlist.fully_keyed = if vlist.fully_keyed() {
+            FullyKeyedState::KnownFullyKeyed
+        } else {
+            FullyKeyedState::KnownMissingKeys
+        };
+        vlist
     }
 
     /// Add [VNode] child.
     pub fn add_child(&mut self, child: VNode) {
-        if self.fully_keyed && !child.has_key() {
+        if self.fully_keyed == FullyKeyedState::KnownFullyKeyed && !child.has_key() {
-            self.fully_keyed = false;
+            self.fully_keyed = FullyKeyedState::KnownMissingKeys;
         }
         self.children.push(child);
     }
@@ -105,11 +92,23 @@ impl VList {
         }
     }
 
-    /// Get a mutable list of children in this vlist.
+    /// Recheck, if the all the children have keys.
-    pub fn children_mut(&mut self) -> ChildrenMut<'_> {
+    ///
-        ChildrenMut {
+    /// You can run this, after modifying the child list through the [DerefMut] implementation of
-            children: &mut self.children,
+    /// [VList], to precompute an internally kept flag, which speeds up reconciliation later.
-            fully_keyed: &mut self.fully_keyed,
+    pub fn recheck_fully_keyed(&mut self) {
+        self.fully_keyed = if self.fully_keyed() {
+            FullyKeyedState::KnownFullyKeyed
+        } else {
+            FullyKeyedState::KnownMissingKeys
+        };
+    }
+
+    pub(crate) fn fully_keyed(&self) -> bool {
+        match self.fully_keyed {
+            FullyKeyedState::KnownFullyKeyed => true,
+            FullyKeyedState::KnownMissingKeys => false,
+            FullyKeyedState::Unknown => self.children.iter().all(|c| c.has_key()),
         }
     }
 }
@@ -122,25 +121,36 @@ mod test {
     #[test]
     fn mutably_change_children() {
         let mut vlist = VList::new();
-        assert!(vlist.fully_keyed, "should start fully keyed");
+        assert_eq!(
+            vlist.fully_keyed,
+            FullyKeyedState::KnownFullyKeyed,
+            "should start fully keyed"
+        );
         // add a child that is keyed
-        let mut children = vlist.children_mut();
+        vlist.add_child(VNode::VTag({
-        children.push(VNode::VTag({
             let mut tag = VTag::new("a");
             tag.key = Some(42u32.into());
-            Box::new(tag)
+            tag.into()
         }));
-        drop(children);
+        assert_eq!(
-        assert!(vlist.fully_keyed, "should still be fully keyed");
+            vlist.fully_keyed,
+            FullyKeyedState::KnownFullyKeyed,
+            "should still be fully keyed"
+        );
         assert_eq!(vlist.len(), 1, "should contain 1 child");
         // now add a child that is not keyed
-        let mut children = vlist.children_mut();
+        vlist.add_child(VNode::VText(VText::new("lorem ipsum")));
-        children.push(VNode::VText(VText::new("lorem ipsum")));
+        assert_eq!(
-        drop(children);
+            vlist.fully_keyed,
-        assert!(
+            FullyKeyedState::KnownMissingKeys,
-            !vlist.fully_keyed,
             "should not be fully keyed, text tags have no key"
         );
+        let _: &mut [VNode] = &mut vlist; // Use deref mut
+        assert_eq!(
+            vlist.fully_keyed,
+            FullyKeyedState::Unknown,
+            "key state should be unknown, since it was potentially modified through children"
+        );
     }
 }