From 12fab0f9f718c881fb543bb2d7c75b7185671416 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 3 Oct 2025 21:23:34 -0500 Subject: [PATCH] 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 --- Cargo.lock | 9 ++ Cargo.toml | 1 + packages/fullstack-server/src/ssr.rs | 99 ++++++++++++++++--- .../fullstack-hydration-order.spec.js | 17 ++++ .../fullstack-hydration-order/Cargo.toml | 17 ++++ .../fullstack-hydration-order/src/main.rs | 70 +++++++++++++ packages/playwright-tests/fullstack.spec.js | 8 +- .../playwright-tests/playwright.config.js | 9 ++ 8 files changed, 215 insertions(+), 15 deletions(-) create mode 100644 packages/playwright-tests/fullstack-hydration-order.spec.js create mode 100644 packages/playwright-tests/fullstack-hydration-order/Cargo.toml create mode 100644 packages/playwright-tests/fullstack-hydration-order/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index e001c70b1..67990fbd1 100644 --- a/Cargo.lock +++ b/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" diff --git a/Cargo.toml b/Cargo.toml index 3cbf63791..05b610569 100644 --- a/Cargo.toml +++ b/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", diff --git a/packages/fullstack-server/src/ssr.rs b/packages/fullstack-server/src/ssr.rs index 2fd161d6a..663d34bd3 100644 --- a/packages/fullstack-server/src/ssr.rs +++ b/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>, + 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( cfg: &ServeConfig, diff --git a/packages/playwright-tests/fullstack-hydration-order.spec.js b/packages/playwright-tests/fullstack-hydration-order.spec.js new file mode 100644 index 000000000..892ef4b01 --- /dev/null +++ b/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"); +}); diff --git a/packages/playwright-tests/fullstack-hydration-order/Cargo.toml b/packages/playwright-tests/fullstack-hydration-order/Cargo.toml new file mode 100644 index 000000000..ba25d3695 --- /dev/null +++ b/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"] diff --git a/packages/playwright-tests/fullstack-hydration-order/src/main.rs b/packages/playwright-tests/fullstack-hydration-order/src/main.rs new file mode 100644 index 000000000..93fbc59de --- /dev/null +++ b/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, ServerFnError> { + let data: Vec = 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, ServerFnError> { + let data: Vec = vec![1.0, 2.0, 3.0]; + Ok(data) +} diff --git a/packages/playwright-tests/fullstack.spec.js b/packages/playwright-tests/fullstack.spec.js index 9453341c7..21536741f 100644 --- a/packages/playwright-tests/fullstack.spec.js +++ b/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"]; diff --git a/packages/playwright-tests/playwright.config.js b/packages/playwright-tests/playwright.config.js index 63aedee0a..1ef300359 100644 --- a/packages/playwright-tests/playwright.config.js +++ b/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