Fix fullstack serialization order for adjacent components (#4600)

* fix serialization order

* add regression test

* split out test into a seperate package

* revert logging

* change order

* opinions on structure

* small wait in hydration order test

---------

Co-authored-by: Jonathan Kelley <jkelleyrtp@gmail.com>

Cargo.lock

@@ -5899,6 +5899,15 @@ dependencies = [
  "tokio",
 ]
 
+[[package]]
+name = "dioxus-playwright-fullstack-hydration-order-test"
+version = "0.1.0"
+dependencies = [
+ "dioxus",
+ "serde",
+ "tokio",
+]
+
 [[package]]
 name = "dioxus-playwright-fullstack-mounted-test"
 version = "0.1.0"

Cargo.toml

@@ -131,6 +131,7 @@ members = [
     "packages/playwright-tests/fullstack-errors",
     "packages/playwright-tests/fullstack-mounted",
     "packages/playwright-tests/fullstack-routing",
+    "packages/playwright-tests/fullstack-hydration-order",
     "packages/playwright-tests/suspense-carousel",
     "packages/playwright-tests/nested-suspense",
     "packages/playwright-tests/cli-optimization",

packages/fullstack-server/src/ssr.rs

@@ -7,7 +7,8 @@ use crate::streaming::{Mount, StreamingRenderer};
 use crate::{document::ServerDocument, ServeConfig};
 use dioxus_cli_config::base_path;
 use dioxus_core::{
-    has_context, DynamicNode, ErrorContext, ScopeId, SuspenseContext, VNode, VirtualDom,
+    has_context, DynamicNode, ErrorContext, ScopeId, SuspenseContext, TemplateNode, VNode,
+    VirtualDom,
 };
 use dioxus_fullstack_core::{history::provide_fullstack_history_context, HttpError, ServerFnError};
 use dioxus_fullstack_core::{HydrationContext, SerializedHydrationData};
@@ -17,7 +18,14 @@ use dioxus_ssr::Renderer;
 use futures_channel::mpsc::Sender;
 use futures_util::{Stream, StreamExt};
 use http::StatusCode;
-use std::{collections::HashMap, fmt::Write, future::Future, rc::Rc, sync::Arc, sync::RwLock};
+use std::{
+    collections::HashMap,
+    fmt::Write,
+    future::Future,
+    iter::Peekable,
+    rc::Rc,
+    sync::{Arc, RwLock},
+};
 use tokio::task::JoinHandle;
 
 use crate::StreamingMode;
@@ -555,23 +563,92 @@ impl SsrRendererPool {
     }
 
     fn take_from_vnode(context: &HydrationContext, vdom: &VirtualDom, vnode: &VNode) {
-        for (dynamic_node_index, dyn_node) in vnode.dynamic_nodes.iter().enumerate() {
-            match dyn_node {
-                DynamicNode::Component(comp) => {
-                    if let Some(scope) = comp.mounted_scope(dynamic_node_index, vnode, vdom) {
-                        Self::take_from_scope(context, vdom, scope.id());
+        let template = &vnode.template;
+        let mut dynamic_nodes_iter = template.node_paths.iter().copied().enumerate().peekable();
+        for (root_idx, node) in template.roots.iter().enumerate() {
+            match node {
+                TemplateNode::Element { .. } => {
+                    // dioxus core runs nodes in an odd order to not mess up template order. We need to match
+                    // that order here
+                    let (start, end) =
+                        match Self::collect_dyn_node_range(&mut dynamic_nodes_iter, root_idx as u8)
+                        {
+                            Some((a, b)) => (a, b),
+                            None => continue,
+                        };
+
+                    let reversed_iter = (start..=end).rev();
+
+                    for dynamic_node_id in reversed_iter {
+                        let dynamic_node = &vnode.dynamic_nodes[dynamic_node_id];
+                        Self::take_from_dynamic_node(
+                            context,
+                            vdom,
+                            vnode,
+                            dynamic_node,
+                            dynamic_node_id,
+                        );
                     }
                 }
-                DynamicNode::Fragment(nodes) => {
-                    for node in nodes {
-                        Self::take_from_vnode(context, vdom, node);
-                    }
+                TemplateNode::Dynamic { id } => {
+                    // Take a dynamic node off the depth first iterator
+                    _ = dynamic_nodes_iter.next().unwrap();
+                    let dynamic_node = &vnode.dynamic_nodes[*id];
+                    Self::take_from_dynamic_node(context, vdom, vnode, dynamic_node, *id);
                 }
                 _ => {}
             }
         }
     }
 
+    fn take_from_dynamic_node(
+        context: &HydrationContext,
+        vdom: &VirtualDom,
+        vnode: &VNode,
+        dyn_node: &DynamicNode,
+        dynamic_node_index: usize,
+    ) {
+        match dyn_node {
+            DynamicNode::Component(comp) => {
+                if let Some(scope) = comp.mounted_scope(dynamic_node_index, vnode, vdom) {
+                    Self::take_from_scope(context, vdom, scope.id());
+                }
+            }
+            DynamicNode::Fragment(nodes) => {
+                for node in nodes {
+                    Self::take_from_vnode(context, vdom, node);
+                }
+            }
+            _ => {}
+        }
+    }
+
+    // This should have the same behavior as the collect_dyn_node_range method in core
+    // Find the index of the first and last dynamic node under a root index
+    fn collect_dyn_node_range(
+        dynamic_nodes: &mut Peekable<impl Iterator<Item = (usize, &'static [u8])>>,
+        root_idx: u8,
+    ) -> Option<(usize, usize)> {
+        let start = match dynamic_nodes.peek() {
+            Some((idx, [first, ..])) if *first == root_idx => *idx,
+            _ => return None,
+        };
+
+        let mut end = start;
+
+        while let Some((idx, p)) =
+            dynamic_nodes.next_if(|(_, p)| matches!(p, [idx, ..] if *idx == root_idx))
+        {
+            if p.len() == 1 {
+                continue;
+            }
+
+            end = idx;
+        }
+
+        Some((start, end))
+    }
+
     /// Render any content before the head of the page.
     pub fn render_head<R: std::fmt::Write>(
         cfg: &ServeConfig,

packages/playwright-tests/fullstack-hydration-order.spec.js

@@ -0,0 +1,17 @@
+// @ts-check
+const { test, expect } = require("@playwright/test");
+
+test("hydration", async ({ page }) => {
+  await page.goto("http://localhost:7979");
+
+  // give time for the page to load and hydrate
+  await page.waitForTimeout(2000);
+
+  // Expect the page to contain a button
+  const button = page.locator("#counter");
+  await expect(button).toContainText("Count 0");
+
+  // Hydration should succeed and clicking the button should increase the count
+  await button.click();
+  await expect(button).toContainText("Count 1");
+});

packages/playwright-tests/fullstack-hydration-order/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+name = "dioxus-playwright-fullstack-hydration-order-test"
+version = "0.1.0"
+edition = "2021"
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+dioxus = { workspace = true, features = ["fullstack"] }
+serde = "1.0.219"
+tokio = { workspace = true, features = ["full"], optional = true }
+
+[features]
+default = []
+server = ["dioxus/server", "dep:tokio"]
+web = ["dioxus/web"]

packages/playwright-tests/fullstack-hydration-order/src/main.rs

@@ -0,0 +1,70 @@
+// Adjacent server components that both use server functions shouldn't cause
+// hydration issues
+// https://github.com/DioxusLabs/dioxus/issues/4595
+
+use dioxus::prelude::*;
+
+fn main() {
+    dioxus::launch(|| rsx! { Home {} });
+}
+
+#[component]
+pub fn Home() -> Element {
+    let mut count = use_signal(|| 0);
+    rsx! {
+        div {
+            MyStrings {}
+            MyFloats {}
+        }
+        button {
+            id: "counter",
+            onclick: move |_| count += 1,
+            "Count {count}"
+        }
+    }
+}
+
+#[component]
+fn MyStrings() -> Element {
+    let strings = use_server_future(get_strings)?;
+    let data = match &*strings.read() {
+        Some(Ok(data)) => data.clone(),
+        _ => vec![],
+    };
+
+    rsx! {
+        div {
+            for string in data.iter() {
+                p { "{string}" }
+            }
+        }
+    }
+}
+#[get("/api/get_strings")]
+pub async fn get_strings() -> Result<Vec<String>, ServerFnError> {
+    let data: Vec<String> = vec!["Hello".to_string(), "World".to_string()];
+    Ok(data)
+}
+
+#[component]
+fn MyFloats() -> Element {
+    let floats = use_server_future(get_floats)?;
+    let data = match &*floats.read() {
+        Some(Ok(data)) => data.clone(),
+        _ => vec![],
+    };
+
+    rsx! {
+        div {
+            for float in data.iter() {
+                p { "{float}" }
+            }
+        }
+    }
+}
+
+#[get("/api/get_floats")]
+pub async fn get_floats() -> Result<Vec<f32>, ServerFnError> {
+    let data: Vec<f32> = vec![1.0, 2.0, 3.0];
+    Ok(data)
+}

packages/playwright-tests/fullstack.spec.js

@@ -104,10 +104,10 @@ test("assets cache correctly", async ({ page }) => {
 
   // Wait for the nested asset image to be loaded
   const nestedAssetImageResponse = await nestedAssetImageFuture;
-  console.log(
-    "Nested Asset Image Response:",
-    nestedAssetImageResponse
-  );
+  // console.log(
+  //   "Nested Asset Image Response:",
+  //   nestedAssetImageResponse
+  // );
   // Make sure the nested asset image cache control header does not contain immutable
   const nestedAssetCacheControl =
     nestedAssetImageResponse.headers()["cache-control"];

packages/playwright-tests/playwright.config.js

@@ -144,6 +144,15 @@ module.exports = defineConfig({
       reuseExistingServer: !process.env.CI,
       stdout: "pipe",
     },
+    {
+      cwd: path.join(process.cwd(), "fullstack-hydration-order"),
+      command:
+        'cargo run --package dioxus-cli --release -- run --force-sequential --web --addr "127.0.0.1" --port 7979',
+      port: 7979,
+      timeout: 50 * 60 * 1000,
+      reuseExistingServer: !process.env.CI,
+      stdout: "pipe",
+    },
     {
       cwd: path.join(process.cwd(), "cli-optimization"),
       // Remove the cache folder for the cli-optimization build to force a full cache reset