Dont shift on normal props updates (#2705)

* don't shift on normal props updates

would remove focus from elements and fire other
dom mutation events we don't need or want

* rename mode -> creation_mode

it's not updated when the component finishes hydration

* rename node_ref to internal_ref for consistency

* fix the bug, should return internal_ref as node_ref gets unset during reconciliation

* also no need to shift during hydration props update

* debug: encode hydration invariant

* fix: next_sibling of descendents not updated

* add test case for regression

* address review and add one more test

packages/yew/src/app_handle.rs

@@ -88,8 +88,10 @@ mod feat_hydration {
                 host.clone(),
                 &mut fragment,
                 NodeRef::default(),
-                props,
+                Rc::clone(&props),
             );
+            #[cfg(debug_assertions)] // Fix trapped next_sibling at the root
+            app.scope.reuse(props, NodeRef::default());
 
             // We remove all remaining nodes, this mimics the clear_element behaviour in
             // mount_with_props.

packages/yew/src/dom_bundle/bcomp.rs

@@ -47,7 +47,7 @@ impl ReconcileTarget for BComp {
     fn shift(&self, next_parent: &Element, next_sibling: NodeRef) -> NodeRef {
         self.scope.shift_node(next_parent.clone(), next_sibling);
 
-        self.node_ref.clone()
+        self.internal_ref.clone()
     }
 }
 
@@ -72,9 +72,9 @@ impl Reconcilable for VComp {
 
         let scope = mountable.mount(
             root,
-            internal_ref.clone(),
             parent_scope,
             parent.to_owned(),
+            internal_ref.clone(),
             next_sibling,
         );
 
@@ -158,8 +158,8 @@ mod feat_hydration {
                 root.clone(),
                 parent_scope,
                 parent.clone(),
-                fragment,
                 internal_ref.clone(),
+                fragment,
             );
 
             (

packages/yew/src/dom_bundle/bsuspense.rs

@@ -174,14 +174,10 @@ impl Reconcilable for VSuspense {
                         vfallback.reconcile_node(root, parent_scope, parent, next_sibling, bundle)
                     }
                     #[cfg(feature = "hydration")]
-                    Fallback::Fragment(fragment) => {
-                        let node_ref = NodeRef::default();
-                        match fragment.front().cloned() {
-                            Some(m) => node_ref.set(Some(m)),
-                            None => node_ref.link(next_sibling),
-                        }
-                        node_ref
-                    }
+                    Fallback::Fragment(fragment) => match fragment.front().cloned() {
+                        Some(m) => NodeRef::new(m),
+                        None => next_sibling,
+                    },
                 }
             }
             // Not suspended, just reconcile the children into the DOM
@@ -256,6 +252,8 @@ mod feat_hydration {
                 detached_parent.append_child(node).unwrap();
             }
 
+            // 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);

packages/yew/src/functional/hooks/use_prepared_state/feat_hydration_ssr.rs

@@ -40,7 +40,7 @@ where
         type Output = SuspensionResult<Option<Rc<T>>>;
 
         fn run(self, ctx: &mut HookContext) -> Self::Output {
-            match ctx.mode {
+            match ctx.creation_mode {
                 RenderMode::Ssr => feat_ssr::use_prepared_state(self.f, self.deps).run(ctx),
                 _ => feat_hydration::use_prepared_state(self.deps).run(ctx),
             }
@@ -82,7 +82,7 @@ where
         type Output = SuspensionResult<Option<Rc<T>>>;
 
         fn run(self, ctx: &mut HookContext) -> Self::Output {
-            match ctx.mode {
+            match ctx.creation_mode {
                 RenderMode::Ssr => {
                     feat_ssr::use_prepared_state_with_suspension(self.f, self.deps).run(ctx)
                 }

packages/yew/src/functional/hooks/use_transitive_state/feat_hydration_ssr.rs

@@ -39,7 +39,7 @@ where
         type Output = SuspensionResult<Option<Rc<T>>>;
 
         fn run(self, ctx: &mut HookContext) -> Self::Output {
-            match ctx.mode {
+            match ctx.creation_mode {
                 RenderMode::Ssr => feat_ssr::use_transitive_state(self.f, self.deps).run(ctx),
                 _ => feat_hydration::use_transitive_state(self.deps).run(ctx),
             }

packages/yew/src/functional/mod.rs

@@ -85,7 +85,7 @@ pub struct HookContext {
     pub(crate) scope: AnyScope,
     #[cfg(any(target_arch = "wasm32", feature = "tokio"))]
     #[cfg(all(feature = "hydration", feature = "ssr"))]
-    mode: RenderMode,
+    creation_mode: RenderMode,
     re_render: ReRender,
 
     states: Vec<Rc<dyn Any>>,
@@ -113,7 +113,7 @@ impl HookContext {
         re_render: ReRender,
         #[cfg(any(target_arch = "wasm32", feature = "tokio"))]
         #[cfg(all(feature = "hydration", feature = "ssr"))]
-        mode: RenderMode,
+        creation_mode: RenderMode,
         #[cfg(any(target_arch = "wasm32", feature = "tokio"))]
         #[cfg(feature = "hydration")]
         prepared_state: Option<&str>,
@@ -124,7 +124,7 @@ impl HookContext {
 
             #[cfg(any(target_arch = "wasm32", feature = "tokio"))]
             #[cfg(all(feature = "hydration", feature = "ssr"))]
-            mode,
+            creation_mode,
 
             states: Vec::new(),
 
@@ -363,7 +363,7 @@ where
                 re_render,
                 #[cfg(any(target_arch = "wasm32", feature = "tokio"))]
                 #[cfg(all(feature = "hydration", feature = "ssr"))]
-                ctx.mode(),
+                ctx.creation_mode(),
                 #[cfg(any(target_arch = "wasm32", feature = "tokio"))]
                 #[cfg(feature = "hydration")]
                 ctx.prepared_state(),

packages/yew/src/html/component/lifecycle.rs

@@ -28,15 +28,15 @@ pub(crate) enum ComponentRenderState {
         root: BSubtree,
         parent: Element,
         next_sibling: NodeRef,
-        node_ref: NodeRef,
+        internal_ref: NodeRef,
     },
     #[cfg(feature = "hydration")]
     Hydration {
+        fragment: Fragment,
+        root: BSubtree,
         parent: Element,
         next_sibling: NodeRef,
-        node_ref: NodeRef,
-        root: BSubtree,
-        fragment: Fragment,
+        internal_ref: NodeRef,
     },
 
     #[cfg(feature = "ssr")]
@@ -54,14 +54,14 @@ impl std::fmt::Debug for ComponentRenderState {
                 root,
                 ref parent,
                 ref next_sibling,
-                ref node_ref,
+                ref internal_ref,
             } => f
                 .debug_struct("ComponentRenderState::Render")
                 .field("bundle", bundle)
                 .field("root", root)
                 .field("parent", parent)
                 .field("next_sibling", next_sibling)
-                .field("node_ref", node_ref)
+                .field("internal_ref", internal_ref)
                 .finish(),
 
             #[cfg(feature = "hydration")]
@@ -69,7 +69,7 @@ impl std::fmt::Debug for ComponentRenderState {
                 ref fragment,
                 ref parent,
                 ref next_sibling,
-                ref node_ref,
+                ref internal_ref,
                 ref root,
             } => f
                 .debug_struct("ComponentRenderState::Hydration")
@@ -77,7 +77,7 @@ impl std::fmt::Debug for ComponentRenderState {
                 .field("root", root)
                 .field("parent", parent)
                 .field("next_sibling", next_sibling)
-                .field("node_ref", node_ref)
+                .field("internal_ref", internal_ref)
                 .finish(),
 
             #[cfg(feature = "ssr")]
@@ -109,7 +109,7 @@ impl ComponentRenderState {
                 bundle.shift(&next_parent, next_next_sibling.clone());
 
                 *parent = next_parent;
-                *next_sibling = next_next_sibling;
+                next_sibling.link(next_next_sibling);
             }
             #[cfg(feature = "hydration")]
             Self::Hydration {
@@ -121,7 +121,7 @@ impl ComponentRenderState {
                 fragment.shift(&next_parent, next_next_sibling.clone());
 
                 *parent = next_parent;
-                *next_sibling = next_next_sibling;
+                next_sibling.link(next_next_sibling);
             }
 
             #[cfg(feature = "ssr")]
@@ -160,7 +160,7 @@ pub(crate) trait Stateful {
     fn as_any_mut(&mut self) -> &mut dyn Any;
 
     #[cfg(feature = "hydration")]
-    fn mode(&self) -> RenderMode;
+    fn creation_mode(&self) -> RenderMode;
 }
 
 impl<COMP> Stateful for CompStateInner<COMP>
@@ -184,8 +184,8 @@ where
     }
 
     #[cfg(feature = "hydration")]
-    fn mode(&self) -> RenderMode {
-        self.context.mode
+    fn creation_mode(&self) -> RenderMode {
+        self.context.creation_mode()
     }
 
     fn flush_messages(&mut self) -> bool {
@@ -246,7 +246,7 @@ impl ComponentState {
     ) -> Self {
         let comp_id = scope.id;
         #[cfg(feature = "hydration")]
-        let mode = {
+        let creation_mode = {
             match initial_render_state {
                 ComponentRenderState::Render { .. } => RenderMode::Render,
                 ComponentRenderState::Hydration { .. } => RenderMode::Hydration,
@@ -259,7 +259,7 @@ impl ComponentState {
             scope,
             props,
             #[cfg(feature = "hydration")]
-            mode,
+            creation_mode,
             #[cfg(feature = "hydration")]
             prepared_state,
         };
@@ -344,24 +344,18 @@ impl Runnable for PropsUpdateRunner {
                 match state.render_state {
                     #[cfg(feature = "csr")]
                     ComponentRenderState::Render {
-                        next_sibling: ref mut current_next_sibling,
-                        ref parent,
-                        ref bundle,
+                        next_sibling: ref current_next_sibling,
                         ..
                     } => {
-                        bundle.shift(parent, next_sibling.clone());
-                        *current_next_sibling = next_sibling;
+                        current_next_sibling.link(next_sibling);
                     }
 
                     #[cfg(feature = "hydration")]
                     ComponentRenderState::Hydration {
-                        next_sibling: ref mut current_next_sibling,
-                        ref parent,
-                        ref fragment,
+                        next_sibling: ref current_next_sibling,
                         ..
                     } => {
-                        fragment.shift(parent, next_sibling.clone());
-                        *current_next_sibling = next_sibling;
+                        current_next_sibling.link(next_sibling);
                     }
 
                     #[cfg(feature = "ssr")]
@@ -399,7 +393,7 @@ impl Runnable for PropsUpdateRunner {
             let schedule_render = {
                 #[cfg(feature = "hydration")]
                 {
-                    if state.inner.mode() == RenderMode::Hydration {
+                    if state.inner.creation_mode() == RenderMode::Hydration {
                         should_render_hydration(props, state)
                     } else {
                         should_render(props, state)
@@ -478,13 +472,13 @@ impl Runnable for DestroyRunner {
                 ComponentRenderState::Render {
                     bundle,
                     ref parent,
-                    ref node_ref,
+                    ref internal_ref,
                     ref root,
                     ..
                 } => {
                     bundle.detach(root, parent, self.parent_to_detach);
 
-                    node_ref.set(None);
+                    internal_ref.set(None);
                 }
                 // We need to detach the hydrate fragment if the component is not hydrated.
                 #[cfg(feature = "hydration")]
@@ -492,12 +486,12 @@ impl Runnable for DestroyRunner {
                     ref root,
                     fragment,
                     ref parent,
-                    ref node_ref,
+                    ref internal_ref,
                     ..
                 } => {
                     fragment.detach(root, parent, self.parent_to_detach);
 
-                    node_ref.set(None);
+                    internal_ref.set(None);
                 }
 
                 #[cfg(feature = "ssr")]
@@ -593,14 +587,17 @@ impl RenderRunner {
                 ref parent,
                 ref root,
                 ref next_sibling,
-                ref node_ref,
+                ref internal_ref,
                 ..
             } => {
                 let scope = state.inner.any_scope();
 
+                #[cfg(feature = "hydration")]
+                next_sibling.debug_assert_not_trapped();
+
                 let new_node_ref =
                     bundle.reconcile(root, &scope, parent, next_sibling.clone(), new_root);
-                node_ref.link(new_node_ref);
+                internal_ref.link(new_node_ref);
 
                 let first_render = !state.has_rendered;
                 state.has_rendered = true;
@@ -619,7 +616,7 @@ impl RenderRunner {
             ComponentRenderState::Hydration {
                 ref mut fragment,
                 ref parent,
-                ref node_ref,
+                ref internal_ref,
                 ref next_sibling,
                 ref root,
             } => {
@@ -644,13 +641,13 @@ impl RenderRunner {
 
                 assert!(fragment.is_empty(), "expected end of component, found node");
 
-                node_ref.link(node);
+                internal_ref.link(node);
 
                 state.render_state = ComponentRenderState::Render {
                     root: root.clone(),
                     bundle,
                     parent: parent.clone(),
-                    node_ref: node_ref.clone(),
+                    internal_ref: internal_ref.clone(),
                     next_sibling: next_sibling.clone(),
                 };
             }

packages/yew/src/html/component/mod.rs

@@ -74,7 +74,7 @@ pub struct Context<COMP: BaseComponent> {
     scope: Scope<COMP>,
     props: Rc<COMP::Properties>,
     #[cfg(feature = "hydration")]
-    mode: RenderMode,
+    creation_mode: RenderMode,
 
     #[cfg(feature = "hydration")]
     prepared_state: Option<String>,
@@ -94,8 +94,8 @@ impl<COMP: BaseComponent> Context<COMP> {
     }
 
     #[cfg(feature = "hydration")]
-    pub(crate) fn mode(&self) -> RenderMode {
-        self.mode
+    pub(crate) fn creation_mode(&self) -> RenderMode {
+        self.creation_mode
     }
 
     /// The component's prepared state

packages/yew/src/html/component/scope.rs

@@ -512,17 +512,19 @@ mod feat_csr {
             root: BSubtree,
             parent: Element,
             next_sibling: NodeRef,
-            node_ref: NodeRef,
+            internal_ref: NodeRef,
             props: Rc<COMP::Properties>,
         ) {
             let bundle = Bundle::new();
-            node_ref.link(next_sibling.clone());
+            internal_ref.link(next_sibling.clone());
+            let stable_next_sibling = NodeRef::default();
+            stable_next_sibling.link(next_sibling);
             let state = ComponentRenderState::Render {
                 bundle,
                 root,
-                node_ref,
+                internal_ref,
                 parent,
-                next_sibling,
+                next_sibling: stable_next_sibling,
             };
 
             scheduler::push_component_create(
@@ -632,7 +634,7 @@ mod feat_hydration {
             root: BSubtree,
             parent: Element,
             fragment: &mut Fragment,
-            node_ref: NodeRef,
+            internal_ref: NodeRef,
             props: Rc<COMP::Properties>,
         ) {
             // This is very helpful to see which component is failing during hydration
@@ -647,8 +649,14 @@ mod feat_hydration {
             let collectable = Collectable::for_component::<COMP>();
 
             let mut fragment = Fragment::collect_between(fragment, &collectable, &parent);
-            node_ref.set(fragment.front().cloned());
-            let next_sibling = NodeRef::default();
+            match fragment.front().cloned() {
+                front @ Some(_) => internal_ref.set(front),
+                None =>
+                {
+                    #[cfg(debug_assertions)]
+                    internal_ref.link(NodeRef::new_debug_trapped())
+                }
+            }
 
             let prepared_state = match fragment
                 .back()
@@ -664,10 +672,10 @@ mod feat_hydration {
             };
 
             let state = ComponentRenderState::Hydration {
-                root,
                 parent,
-                node_ref,
-                next_sibling,
+                root,
+                internal_ref,
+                next_sibling: NodeRef::new_debug_trapped(),
                 fragment,
             };
 

packages/yew/src/html/mod.rs

@@ -172,6 +172,43 @@ mod feat_csr {
     }
 }
 
+#[cfg(feature = "hydration")]
+mod feat_hydration {
+    use super::*;
+
+    #[cfg(debug_assertions)]
+    thread_local! {
+        // A special marker element that should not be referenced
+        static TRAP: Node = gloo::utils::document().create_element("div").unwrap().into();
+    }
+
+    impl NodeRef {
+        // A new "placeholder" node ref that should not be accessed
+        #[inline]
+        pub(crate) fn new_debug_trapped() -> Self {
+            #[cfg(debug_assertions)]
+            {
+                Self::new(TRAP.with(|trap| trap.clone()))
+            }
+            #[cfg(not(debug_assertions))]
+            {
+                Self::default()
+            }
+        }
+
+        #[inline]
+        pub(crate) fn debug_assert_not_trapped(&self) {
+            #[cfg(debug_assertions)]
+            TRAP.with(|trap| {
+                assert!(
+                    self.get().as_ref() != Some(trap),
+                    "should not use a trapped node ref"
+                )
+            })
+        }
+    }
+}
+
 /// Render children into a DOM node that exists outside the hierarchy of the parent
 /// component.
 /// ## Relevant examples

packages/yew/src/suspense/component.rs

@@ -56,7 +56,7 @@ mod feat_csr_ssr {
                 use crate::callback::Callback;
                 use crate::html::RenderMode;
 
-                match ctx.mode() {
+                match ctx.creation_mode() {
                     RenderMode::Hydration => {
                         let link = ctx.link().clone();
                         let (s, handle) = Suspension::new();

packages/yew/src/virtual_dom/vcomp.rs

@@ -57,9 +57,9 @@ pub(crate) trait Mountable {
     fn mount(
         self: Box<Self>,
         root: &BSubtree,
-        node_ref: NodeRef,
         parent_scope: &AnyScope,
         parent: Element,
+        internal_ref: NodeRef,
         next_sibling: NodeRef,
     ) -> Box<dyn Scoped>;
 
@@ -80,8 +80,8 @@ pub(crate) trait Mountable {
         root: BSubtree,
         parent_scope: &AnyScope,
         parent: Element,
+        internal_ref: NodeRef,
         fragment: &mut Fragment,
-        node_ref: NodeRef,
     ) -> Box<dyn Scoped>;
 }
 
@@ -107,13 +107,13 @@ impl<COMP: BaseComponent> Mountable for PropsWrapper<COMP> {
     fn mount(
         self: Box<Self>,
         root: &BSubtree,
-        node_ref: NodeRef,
         parent_scope: &AnyScope,
         parent: Element,
+        internal_ref: NodeRef,
         next_sibling: NodeRef,
     ) -> Box<dyn Scoped> {
         let scope: Scope<COMP> = Scope::new(Some(parent_scope.clone()));
-        scope.mount_in_place(root.clone(), parent, next_sibling, node_ref, self.props);
+        scope.mount_in_place(root.clone(), parent, next_sibling, internal_ref, self.props);
 
         Box::new(scope)
     }
@@ -146,11 +146,11 @@ impl<COMP: BaseComponent> Mountable for PropsWrapper<COMP> {
         root: BSubtree,
         parent_scope: &AnyScope,
         parent: Element,
+        internal_ref: NodeRef,
         fragment: &mut Fragment,
-        node_ref: NodeRef,
     ) -> Box<dyn Scoped> {
         let scope: Scope<COMP> = Scope::new(Some(parent_scope.clone()));
-        scope.hydrate_in_place(root, parent, fragment, node_ref, self.props);
+        scope.hydrate_in_place(root, parent, fragment, internal_ref, self.props);
 
         Box::new(scope)
     }

packages/yew/tests/hydration.rs

@@ -977,3 +977,54 @@ async fn hydration_props_blocked_until_hydrated() {
     let result = obtain_result_by_id("output");
     assert_eq!(result.as_str(), r#"<div>0</div><div>1</div><div>2</div>"#);
 }
+
+#[wasm_bindgen_test]
+async fn hydrate_empty() {
+    #[function_component]
+    fn Updating() -> Html {
+        let trigger = use_state(|| false);
+        {
+            let trigger = trigger.clone();
+            use_effect_with_deps(
+                move |_| {
+                    trigger.set(true);
+                    || {}
+                },
+                (),
+            );
+        }
+        if *trigger {
+            html! { <div>{"after"}</div> }
+        } else {
+            html! { <div>{"before"}</div> }
+        }
+    }
+    #[function_component]
+    fn Empty() -> Html {
+        html! { <></> }
+    }
+    #[function_component]
+    fn App() -> Html {
+        html! {
+            <>
+                <Updating />
+                <Empty />
+                <Updating />
+            </>
+        }
+    }
+    let s = ServerRenderer::<App>::new().render().await;
+
+    let output_element = gloo::utils::document()
+        .query_selector("#output")
+        .unwrap()
+        .unwrap();
+
+    output_element.set_inner_html(&s);
+
+    Renderer::<App>::with_root(output_element).hydrate();
+    sleep(Duration::from_millis(50)).await;
+
+    let result = obtain_result_by_id("output");
+    assert_eq!(result.as_str(), r#"<div>after</div><div>after</div>"#);
+}

packages/yew/tests/layout.rs

@@ -0,0 +1,80 @@
+#![cfg(target_arch = "wasm32")]
+
+mod common;
+
+wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);
+
+use std::time::Duration;
+
+use common::obtain_result;
+use gloo::timers::future::sleep;
+use wasm_bindgen_futures::spawn_local;
+use wasm_bindgen_test::*;
+use yew::prelude::*;
+
+#[wasm_bindgen_test]
+async fn change_nested_after_append() {
+    #[function_component]
+    fn Nested() -> Html {
+        let delayed_trigger = use_state(|| true);
+
+        {
+            let delayed_trigger = delayed_trigger.clone();
+            use_effect_with_deps(
+                move |_| {
+                    spawn_local(async move {
+                        sleep(Duration::from_millis(50)).await;
+                        delayed_trigger.set(false);
+                    });
+                    || {}
+                },
+                (),
+            );
+        }
+
+        if *delayed_trigger {
+            html! { <div>{"failure"}</div> }
+        } else {
+            html! { <><i></i><span id="result">{"success"}</span></> }
+        }
+    }
+
+    #[function_component]
+    fn Top() -> Html {
+        html! { <Nested /> }
+    }
+
+    #[function_component]
+    fn App() -> Html {
+        let show_bottom = use_state_eq(|| false);
+
+        {
+            let show_bottom = show_bottom.clone();
+
+            use_effect_with_deps(
+                move |_| {
+                    show_bottom.set(true);
+                    || {}
+                },
+                (),
+            );
+        }
+
+        html! {
+            <>
+                <Top />
+                if *show_bottom {
+                    <div>{"<div>Bottom</div>"}</div>
+                }
+            </>
+        }
+    }
+
+    yew::Renderer::<App>::with_root(gloo_utils::document().get_element_by_id("output").unwrap())
+        .render();
+
+    sleep(Duration::from_millis(100)).await;
+
+    let result = obtain_result();
+    assert_eq!(result.as_str(), "success");
+}