Fix internally tracked sibling positions earlier when hydrating (#3914)

* fix internally tracked sibling positions earlier

previously, the fixup was done in the first "real" render, now this
happens immediately in the render that transitions from
ComponentRenderState::Hydration to ComponentRenderState::Render.

We do so by threading through a reference to the sibling that needs
a fix

* test case for element with early change in render order
   harden test case by using an extra inner component

Author: WorldSEnder

packages/yew/src/app_handle.rs

@@ -5,7 +5,7 @@ use std::rc::Rc;
 
 use web_sys::Element;
 
-use crate::dom_bundle::{BSubtree, DomSlot, DynamicDomSlot};
+use crate::dom_bundle::{BSubtree, DomSlot};
 use crate::html::{BaseComponent, Scope, Scoped};
 
 /// An instance of an application.
@@ -34,13 +34,9 @@ where
             scope: Scope::new(None),
         };
         let hosting_root = BSubtree::create_root(&host);
-        app.scope.mount_in_place(
-            hosting_root,
-            host,
-            DomSlot::at_end(),
-            DynamicDomSlot::new_debug_trapped(),
-            props,
-        );
+        let _ = app
+            .scope
+            .mount_in_place(hosting_root, host, DomSlot::at_end(), props);
 
         app
     }
@@ -110,15 +106,17 @@ mod feat_hydration {
             let mut fragment = Fragment::collect_children(&host);
             let hosting_root = BSubtree::create_root(&host);
 
+            let mut previous_next_sibling = None;
             app.scope.hydrate_in_place(
                 hosting_root,
                 host.clone(),
                 &mut fragment,
-                DynamicDomSlot::new_debug_trapped(),
                 Rc::clone(&props),
+                &mut previous_next_sibling,
             );
-            #[cfg(debug_assertions)] // Fix trapped next_sibling at the root
-            app.scope.reuse(props, DomSlot::at_end());
+            if let Some(previous_next_sibling) = previous_next_sibling {
+                previous_next_sibling.reassign(DomSlot::at_end());
+            }
 
             // We remove all remaining nodes, this mimics the clear_element behaviour in
             // mount_with_props.

packages/yew/src/dom_bundle/bcomp.rs

@@ -63,23 +63,16 @@ impl Reconcilable for VComp {
             key,
             ..
         } = self;
-        let internal_ref = DynamicDomSlot::new_debug_trapped();
-
-        let scope = mountable.mount(
-            root,
-            parent_scope,
-            parent.to_owned(),
-            slot,
-            internal_ref.clone(),
-        );
+
+        let (scope, internal_ref) = mountable.mount(root, parent_scope, parent.to_owned(), slot);
 
         (
             internal_ref.to_position(),
             BComp {
                 type_id,
+                scope,
                 own_position: internal_ref,
                 key,
-                scope,
             },
         )
     }
@@ -131,27 +124,27 @@ mod feat_hydration {
             parent_scope: &AnyScope,
             parent: &Element,
             fragment: &mut Fragment,
+            prev_next_sibling: &mut Option<DynamicDomSlot>,
         ) -> Self::Bundle {
             let VComp {
                 type_id,
                 mountable,
                 key,
                 ..
             } = self;
-            let internal_ref = DynamicDomSlot::new_debug_trapped();
 
-            let scoped = mountable.hydrate(
+            let (scope, own_slot) = mountable.hydrate(
                 root.clone(),
                 parent_scope,
                 parent.clone(),
-                internal_ref.clone(),
                 fragment,
+                prev_next_sibling,
             );
 
             BComp {
                 type_id,
-                scope: scoped,
-                own_position: internal_ref,
+                scope,
+                own_position: own_slot,
                 key,
             }
         }

packages/yew/src/dom_bundle/blist.rs

@@ -494,7 +494,7 @@ impl Reconcilable for VList {
 #[cfg(feature = "hydration")]
 mod feat_hydration {
     use super::*;
-    use crate::dom_bundle::{Fragment, Hydratable};
+    use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable};
 
     impl Hydratable for VList {
         fn hydrate(
@@ -503,13 +503,14 @@ mod feat_hydration {
             parent_scope: &AnyScope,
             parent: &Element,
             fragment: &mut Fragment,
+            prev_next_sibling: &mut Option<DynamicDomSlot>,
         ) -> Self::Bundle {
             let (key, fully_keyed, vchildren) = self.split_for_blist();
 
             let mut children = Vec::with_capacity(vchildren.len());
 
             for child in vchildren.into_iter() {
-                let child = child.hydrate(root, parent_scope, parent, fragment);
+                let child = child.hydrate(root, parent_scope, parent, fragment, prev_next_sibling);
 
                 children.push(child);
             }

packages/yew/src/dom_bundle/bnode.rs

@@ -267,7 +267,7 @@ impl fmt::Debug for BNode {
 #[cfg(feature = "hydration")]
 mod feat_hydration {
     use super::*;
-    use crate::dom_bundle::{Fragment, Hydratable};
+    use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable};
 
     impl Hydratable for VNode {
         fn hydrate(
@@ -276,17 +276,20 @@ mod feat_hydration {
             parent_scope: &AnyScope,
             parent: &Element,
             fragment: &mut Fragment,
+            prev_next_sibling: &mut Option<DynamicDomSlot>,
         ) -> Self::Bundle {
             match self {
                 VNode::VTag(vtag) => RcExt::unwrap_or_clone(vtag)
-                    .hydrate(root, parent_scope, parent, fragment)
+                    .hydrate(root, parent_scope, parent, fragment, prev_next_sibling)
+                    .into(),
+                VNode::VText(vtext) => vtext
+                    .hydrate(root, parent_scope, parent, fragment, prev_next_sibling)
                     .into(),
-                VNode::VText(vtext) => vtext.hydrate(root, parent_scope, parent, fragment).into(),
                 VNode::VComp(vcomp) => RcExt::unwrap_or_clone(vcomp)
-                    .hydrate(root, parent_scope, parent, fragment)
+                    .hydrate(root, parent_scope, parent, fragment, prev_next_sibling)
                     .into(),
                 VNode::VList(vlist) => RcExt::unwrap_or_clone(vlist)
-                    .hydrate(root, parent_scope, parent, fragment)
+                    .hydrate(root, parent_scope, parent, fragment, prev_next_sibling)
                     .into(),
                 // You cannot hydrate a VRef.
                 VNode::VRef(_) => {
@@ -303,9 +306,11 @@ mod feat_hydration {
                     )
                 }
                 VNode::VSuspense(vsuspense) => RcExt::unwrap_or_clone(vsuspense)
-                    .hydrate(root, parent_scope, parent, fragment)
+                    .hydrate(root, parent_scope, parent, fragment, prev_next_sibling)
+                    .into(),
+                VNode::VRaw(vraw) => vraw
+                    .hydrate(root, parent_scope, parent, fragment, prev_next_sibling)
                     .into(),
-                VNode::VRaw(vraw) => vraw.hydrate(root, parent_scope, parent, fragment).into(),
             }
         }
     }

packages/yew/src/dom_bundle/braw.rs

@@ -142,7 +142,7 @@ impl Reconcilable for VRaw {
 #[cfg(feature = "hydration")]
 mod feat_hydration {
     use super::*;
-    use crate::dom_bundle::{Fragment, Hydratable};
+    use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable};
     use crate::virtual_dom::Collectable;
 
     impl Hydratable for VRaw {
@@ -152,15 +152,24 @@ mod feat_hydration {
             _parent_scope: &AnyScope,
             parent: &Element,
             fragment: &mut Fragment,
+            prev_next_sibling: &mut Option<DynamicDomSlot>,
         ) -> Self::Bundle {
             let collectable = Collectable::Raw;
             let fallback_fragment = Fragment::collect_between(fragment, &collectable, parent);
+            let first_child = fallback_fragment.iter().next().cloned();
+
+            if let (Some(first_child), prev_next_sibling) = (&first_child, prev_next_sibling) {
+                if let Some(prev_next_sibling) = prev_next_sibling {
+                    prev_next_sibling.reassign(DomSlot::at(first_child.clone()));
+                }
+                *prev_next_sibling = None;
+            }
 
             let Self { html } = self;
 
             BRaw {
                 children_count: fallback_fragment.len(),
-                reference: fallback_fragment.iter().next().cloned(),
+                reference: first_child,
                 html,
             }
         }

packages/yew/src/dom_bundle/bsuspense.rs

@@ -224,7 +224,7 @@ impl Reconcilable for VSuspense {
 #[cfg(feature = "hydration")]
 mod feat_hydration {
     use super::*;
-    use crate::dom_bundle::{Fragment, Hydratable};
+    use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable};
     use crate::virtual_dom::Collectable;
 
     impl Hydratable for VSuspense {
@@ -234,6 +234,7 @@ mod feat_hydration {
             parent_scope: &AnyScope,
             parent: &Element,
             fragment: &mut Fragment,
+            previous_next_sibling: &mut Option<DynamicDomSlot>,
         ) -> Self::Bundle {
             let detached_parent = document()
                 .create_element("div")
@@ -250,9 +251,13 @@ mod feat_hydration {
 
             // Even if initially suspended, these children correspond to the first non-suspended
             // content Refer to VSuspense::render_to_string
-            let children_bundle =
-                self.children
-                    .hydrate(root, parent_scope, &detached_parent, &mut nodes);
+            let children_bundle = self.children.hydrate(
+                root,
+                parent_scope,
+                &detached_parent,
+                &mut nodes,
+                previous_next_sibling,
+            );
 
             // We trim all leading text nodes before checking as it's likely these are whitespaces.
             nodes.trim_start_text_nodes();

packages/yew/src/dom_bundle/btag/mod.rs

@@ -337,7 +337,7 @@ mod feat_hydration {
     use web_sys::Node;
 
     use super::*;
-    use crate::dom_bundle::{node_type_str, Fragment, Hydratable};
+    use crate::dom_bundle::{node_type_str, DynamicDomSlot, Fragment, Hydratable};
 
     impl Hydratable for VTag {
         fn hydrate(
@@ -346,6 +346,7 @@ mod feat_hydration {
             parent_scope: &AnyScope,
             _parent: &Element,
             fragment: &mut Fragment,
+            prev_next_sibling: &mut Option<DynamicDomSlot>,
         ) -> Self::Bundle {
             let tag_name = self.tag().to_owned();
 
@@ -412,7 +413,12 @@ mod feat_hydration {
                 }
                 VTagInner::Other { children, tag } => {
                     let mut nodes = Fragment::collect_children(&el);
-                    let child_bundle = children.hydrate(root, parent_scope, &el, &mut nodes);
+                    let mut prev_next_child = None;
+                    let child_bundle =
+                        children.hydrate(root, parent_scope, &el, &mut nodes, &mut prev_next_child);
+                    if let Some(prev_next_child) = prev_next_child {
+                        prev_next_child.reassign(DomSlot::at_end());
+                    }
 
                     nodes.trim_start_text_nodes();
 
@@ -423,6 +429,10 @@ mod feat_hydration {
             };
 
             node_ref.set(Some((*el).clone()));
+            if let Some(prev_next_sibling) = prev_next_sibling {
+                prev_next_sibling.reassign(DomSlot::at((*el).clone()));
+            }
+            *prev_next_sibling = None;
 
             BTag {
                 inner,

packages/yew/src/dom_bundle/btext.rs

@@ -92,7 +92,7 @@ mod feat_hydration {
     use web_sys::Node;
 
     use super::*;
-    use crate::dom_bundle::{Fragment, Hydratable};
+    use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable};
 
     impl Hydratable for VText {
         fn hydrate(
@@ -101,9 +101,19 @@ mod feat_hydration {
             _parent_scope: &AnyScope,
             parent: &Element,
             fragment: &mut Fragment,
+            previous_next_sibling: &mut Option<DynamicDomSlot>,
         ) -> Self::Bundle {
-            let next_sibling = if let Some(m) = fragment.front().cloned() {
-                // better safe than sorry.
+            let create_at = |next_sibling: Option<Node>, text: AttrValue| {
+                // If there are multiple text nodes placed back-to-back in SSR, it may be parsed as
+                // a single text node by browser, hence we need to add extra text
+                // nodes here if the next node is not a text node. Similarly, the
+                // value of the text node may be a combination of multiple VText
+                // vnodes. So we always need to override their values.
+                let text_node = document().create_text_node(text.as_ref());
+                DomSlot::create(next_sibling).insert(parent, &text_node);
+                BText { text, text_node }
+            };
+            let btext = if let Some(m) = fragment.front().cloned() {
                 if m.node_type() == Node::TEXT_NODE {
                     let m = m.unchecked_into::<TextNode>();
                     // pop current node.
@@ -117,27 +127,21 @@ mod feat_hydration {
                     // Please see the next comment for a detailed explanation.
                     m.set_node_value(Some(self.text.as_ref()));
 
-                    return BText {
+                    BText {
                         text: self.text,
                         text_node: m,
-                    };
+                    }
+                } else {
+                    create_at(Some(m), self.text)
                 }
-                Some(m)
             } else {
-                fragment.sibling_at_end().cloned()
+                create_at(fragment.sibling_at_end().cloned(), self.text)
             };
-
-            // If there are multiple text nodes placed back-to-back in SSR, it may be parsed as a
-            // single text node by browser, hence we need to add extra text nodes here
-            // if the next node is not a text node. Similarly, the value of the text
-            // node may be a combination of multiple VText vnodes. So we always need to
-            // override their values.
-            let text_node = document().create_text_node("");
-            DomSlot::create(next_sibling).insert(parent, &text_node);
-            BText {
-                text: "".into(),
-                text_node,
+            if let Some(previous_next_sibling) = previous_next_sibling {
+                previous_next_sibling.reassign(DomSlot::at(btext.text_node.clone().into()));
             }
+            *previous_next_sibling = None;
+            btext
         }
     }
 }

packages/yew/src/dom_bundle/mod.rs

@@ -94,8 +94,9 @@ mod feat_hydration {
             parent: &Element,
             fragment: &mut Fragment,
             node: VNode,
+            previous_next_sibling: &mut Option<DynamicDomSlot>,
         ) -> Self {
-            let bundle = node.hydrate(root, parent_scope, parent, fragment);
+            let bundle = node.hydrate(root, parent_scope, parent, fragment, previous_next_sibling);
             Self(bundle)
         }
     }