From 65940cbefa238970a319ade8d8e3ea2b43771722 Mon Sep 17 00:00:00 2001 From: Greg Johnston Date: Fri, 19 Dec 2025 10:42:44 -0500 Subject: [PATCH] fix: do not show Transition fallback on 2nd change if 1st change resolved synchronously (closes #4492, closes #3868) (#4495) --- examples/regression/Cargo.toml | 2 +- .../e2e/features/issue_4492.feature | 38 ++++++ .../regression/e2e/tests/fixtures/action.rs | 6 + .../regression/e2e/tests/fixtures/check.rs | 10 +- .../e2e/tests/fixtures/world/action_steps.rs | 15 +++ .../e2e/tests/fixtures/world/check_steps.rs | 11 ++ examples/regression/src/app.rs | 4 +- examples/regression/src/issue_4492.rs | 114 ++++++++++++++++++ examples/regression/src/lib.rs | 1 + leptos/src/suspense_component.rs | 42 +++++-- leptos/src/transition.rs | 21 ++-- leptos_server/src/once_resource.rs | 18 ++- .../async_derived/arc_async_derived.rs | 27 ++++- 13 files changed, 280 insertions(+), 29 deletions(-) create mode 100644 examples/regression/e2e/features/issue_4492.feature create mode 100644 examples/regression/src/issue_4492.rs diff --git a/examples/regression/Cargo.toml b/examples/regression/Cargo.toml index fb5a79b6c..39b160d7c 100644 --- a/examples/regression/Cargo.toml +++ b/examples/regression/Cargo.toml @@ -17,7 +17,7 @@ leptos_router = { path = "../../router" } serde = { version = "1.0", features = ["derive"] } thiserror = "2.0.12" tokio = { version = "1.39", features = [ "rt-multi-thread", "macros", "time" ], optional = true } -wasm-bindgen = "0.2.92" +wasm-bindgen = "0.2.106" [features] hydrate = [ diff --git a/examples/regression/e2e/features/issue_4492.feature b/examples/regression/e2e/features/issue_4492.feature new file mode 100644 index 000000000..dd651d17d --- /dev/null +++ b/examples/regression/e2e/features/issue_4492.feature @@ -0,0 +1,38 @@ +@check_issue_4492 +Feature: Regression test for issue #4492 + + Scenario: Scenario A should show Loading once on first load. + Given I see the app + And I can access regression test 4492 + When I click the button a-toggle + Then I see a-result has the text Loading... + When I wait 100ms + Then I see a-result has the text 0 + When I click the button a-button + Then I see a-result has the text 0 + When I wait 100ms + Then I see a-result has the text 1 + + Scenario: Scenario B should never show Loading + Given I see the app + And I can access regression test 4492 + When I click the button b-toggle + Then I see b-result has the text 0 + When I click the button b-button + Then I see b-result has the text 0 + When I wait 100ms + Then I see b-result has the text 1 + When I click the button b-button + Then I see b-result has the text 1 + When I wait 100ms + Then I see b-result has the text 2 + + Scenario: Scenario C should never show Loading + Given I see the app + And I can access regression test 4492 + When I click the button c-toggle + Then I see c-result has the text 0 + When I click the button c-button + Then I see c-result has the text 42 + When I wait 100ms + Then I see c-result has the text 1 diff --git a/examples/regression/e2e/tests/fixtures/action.rs b/examples/regression/e2e/tests/fixtures/action.rs index 7f4c75adf..fb9f2d869 100644 --- a/examples/regression/e2e/tests/fixtures/action.rs +++ b/examples/regression/e2e/tests/fixtures/action.rs @@ -15,3 +15,9 @@ pub async fn click_link(client: &Client, text: &str) -> Result<()> { link.click().await?; Ok(()) } + +pub async fn click_button(client: &Client, id: &str) -> Result<()> { + let btn = find::element_by_id(&client, &id).await?; + btn.click().await?; + Ok(()) +} diff --git a/examples/regression/e2e/tests/fixtures/check.rs b/examples/regression/e2e/tests/fixtures/check.rs index e021171f4..aecdf324d 100644 --- a/examples/regression/e2e/tests/fixtures/check.rs +++ b/examples/regression/e2e/tests/fixtures/check.rs @@ -7,7 +7,15 @@ pub async fn result_text_is( client: &Client, expected_text: &str, ) -> Result<()> { - let actual = find::text_at_id(client, "result").await?; + element_text_is(client, "result", expected_text).await +} + +pub async fn element_text_is( + client: &Client, + id: &str, + expected_text: &str, +) -> Result<()> { + let actual = find::text_at_id(client, id).await?; assert_eq!(&actual, expected_text); Ok(()) } diff --git a/examples/regression/e2e/tests/fixtures/world/action_steps.rs b/examples/regression/e2e/tests/fixtures/world/action_steps.rs index c0f160dc3..4c908fe94 100644 --- a/examples/regression/e2e/tests/fixtures/world/action_steps.rs +++ b/examples/regression/e2e/tests/fixtures/world/action_steps.rs @@ -20,6 +20,14 @@ async fn i_select_the_link(world: &mut AppWorld, text: String) -> Result<()> { Ok(()) } +#[when(regex = "^I click the button (.*)$")] +async fn i_click_the_button(world: &mut AppWorld, id: String) -> Result<()> { + let client = &world.client; + action::click_button(client, &id).await?; + + Ok(()) +} + #[given(expr = "I select the following links")] #[when(expr = "I select the following links")] async fn i_select_the_following_links( @@ -54,3 +62,10 @@ async fn i_go_back(world: &mut AppWorld) -> Result<()> { Ok(()) } + +#[when(regex = r"^I wait (\d+)ms$")] +async fn i_wait_ms(_world: &mut AppWorld, ms: u64) -> Result<()> { + tokio::time::sleep(std::time::Duration::from_millis(ms)).await; + + Ok(()) +} diff --git a/examples/regression/e2e/tests/fixtures/world/check_steps.rs b/examples/regression/e2e/tests/fixtures/world/check_steps.rs index 51255fedc..c5b6236a0 100644 --- a/examples/regression/e2e/tests/fixtures/world/check_steps.rs +++ b/examples/regression/e2e/tests/fixtures/world/check_steps.rs @@ -19,6 +19,17 @@ async fn i_see_the_result_is_the_string( Ok(()) } +#[then(regex = r"^I see ([\w-]+) has the text (.*)$")] +async fn i_see_element_has_text( + world: &mut AppWorld, + id: String, + text: String, +) -> Result<()> { + let client = &world.client; + check::element_text_is(client, &id, &text).await?; + Ok(()) +} + #[then(regex = r"^I see the navbar$")] async fn i_see_the_navbar(world: &mut AppWorld) -> Result<()> { let client = &world.client; diff --git a/examples/regression/src/app.rs b/examples/regression/src/app.rs index c2a3f85ba..a44f05b04 100644 --- a/examples/regression/src/app.rs +++ b/examples/regression/src/app.rs @@ -1,7 +1,7 @@ use crate::{ issue_4005::Routes4005, issue_4088::Routes4088, issue_4217::Routes4217, issue_4285::Routes4285, issue_4296::Routes4296, issue_4324::Routes4324, - pr_4015::Routes4015, pr_4091::Routes4091, + issue_4492::Routes4492, pr_4015::Routes4015, pr_4091::Routes4091, }; use leptos::prelude::*; use leptos_meta::{MetaTags, *}; @@ -48,6 +48,7 @@ pub fn App() -> impl IntoView { + @@ -75,6 +76,7 @@ fn HomePage() -> impl IntoView {
  • "4285"
  • "4296"
  • "4324"
    • +
  • "4492"
    • } diff --git a/examples/regression/src/issue_4492.rs b/examples/regression/src/issue_4492.rs new file mode 100644 index 000000000..b309edba0 --- /dev/null +++ b/examples/regression/src/issue_4492.rs @@ -0,0 +1,114 @@ +use leptos::prelude::*; +#[allow(unused_imports)] +use leptos_router::{ + components::Route, path, MatchNestedRoutes, NavigateOptions, +}; + +#[component] +pub fn Routes4492() -> impl MatchNestedRoutes + Clone { + view! { + + } + .into_inner() +} + +#[component] +fn Issue4492() -> impl IntoView { + let show_a = RwSignal::new(false); + let show_b = RwSignal::new(false); + let show_c = RwSignal::new(false); + + view! { + + + + + + + + + + + + + + } +} + +#[component] +fn ScenarioA() -> impl IntoView { + // scenario A: one truly-async resource is read on click + let counter = RwSignal::new(0); + let resource = Resource::new( + move || counter.get(), + |count| async move { + sleep(50).await.unwrap(); + count + }, + ); + view! { + "Loading..."

    }> +

    {resource}

    +     + + } +} + +#[component] +fn ScenarioB() -> impl IntoView { + // scenario B: resource immediately available first time, then after 250ms + let counter = RwSignal::new(0); + let resource = Resource::new( + move || counter.get(), + |count| async move { + if count == 0 { + count + } else { + sleep(50).await.unwrap(); + count + } + }, + ); + view! { + "Loading..."

    }> +

    {resource}

    +     + + } +} + +#[component] +fn ScenarioC() -> impl IntoView { + // scenario C: not even a resource on the first run, just a value + // see https://github.com/leptos-rs/leptos/issues/3868 + let counter = RwSignal::new(0); + let s_res = StoredValue::new(None::>); + let resource = move || { + let count = counter.get(); + if count == 0 { + count + } else { + let r = s_res.get_value().unwrap_or_else(|| { + let res = ArcLocalResource::new(move || async move { + sleep(50).await.unwrap(); + count + }); + s_res.set_value(Some(res.clone())); + res + }); + r.get().unwrap_or(42) + } + }; + view! { + "Loading..."

    }> +

    {resource}

    +     + + } +} + +#[server] +async fn sleep(ms: u64) -> Result<(), ServerFnError> { + tokio::time::sleep(std::time::Duration::from_millis(ms)).await; + Ok(()) +} diff --git a/examples/regression/src/lib.rs b/examples/regression/src/lib.rs index acb5aecee..8d2ed835b 100644 --- a/examples/regression/src/lib.rs +++ b/examples/regression/src/lib.rs @@ -5,6 +5,7 @@ mod issue_4217; mod issue_4285; mod issue_4296; mod issue_4324; +mod issue_4492; mod pr_4015; mod pr_4091; diff --git a/leptos/src/suspense_component.rs b/leptos/src/suspense_component.rs index 242f52c98..d14affabe 100644 --- a/leptos/src/suspense_component.rs +++ b/leptos/src/suspense_component.rs @@ -15,7 +15,10 @@ use reactive_graph::{ effect::RenderEffect, owner::{provide_context, use_context, Owner}, signal::ArcRwSignal, - traits::{Dispose, Get, Read, ReadUntracked, Track, With, WriteValue}, + traits::{ + Dispose, Get, Read, ReadUntracked, Track, With, WithUntracked, + WriteValue, + }, }; use slotmap::{DefaultKey, SlotMap}; use std::sync::{Arc, Mutex}; @@ -119,14 +122,19 @@ where provide_context(SuspenseContext { tasks: tasks.clone(), }); - let none_pending = ArcMemo::new(move |prev: Option<&bool>| { - tasks.track(); - if prev.is_none() && starts_local { - false - } else { - tasks.with(SlotMap::is_empty) + let none_pending = ArcMemo::new({ + let tasks = tasks.clone(); + move |prev: Option<&bool>| { + tasks.track(); + if prev.is_none() && starts_local { + false + } else { + tasks.with(SlotMap::is_empty) + } } }); + let has_tasks = + Arc::new(move || !tasks.with_untracked(SlotMap::is_empty)); OwnedView::new(SuspenseBoundary:: { id, @@ -134,6 +142,7 @@ where fallback, children, error_boundary_parent, + has_tasks, }) }) } @@ -156,6 +165,7 @@ pub(crate) struct SuspenseBoundary { pub fallback: Fal, pub children: Chil, pub error_boundary_parent: Option, + pub has_tasks: Arc bool + Send + Sync>, } impl Render @@ -192,12 +202,26 @@ where outer_owner.clone(), ); - if let Some(mut state) = prev { + let state = if let Some(mut state) = prev { this.rebuild(&mut state); state } else { this.build() + }; + + if nth_run == 1 && !(self.has_tasks)() { + // if this is the first run, and there are no pending resources at this point, + // it means that there were no actually-async resources read while rendering the children + // this means that we're effectively on the settled second run: none_pending + // won't change false => true and cause this to rerender (and therefore increment nth_run) + // + // we increment it manually here so that future resource changes won't cause the transition fallback + // to be displayed for the first time + // see https://github.com/leptos-rs/leptos/issues/3868, https://github.com/leptos-rs/leptos/issues/4492 + nth_run += 1; } + + state }) } @@ -235,6 +259,7 @@ where fallback, children, error_boundary_parent, + has_tasks, } = self; SuspenseBoundary { id, @@ -242,6 +267,7 @@ where fallback, children: children.add_any_attr(attr), error_boundary_parent, + has_tasks, } } } diff --git a/leptos/src/transition.rs b/leptos/src/transition.rs index c88682427..72a39d588 100644 --- a/leptos/src/transition.rs +++ b/leptos/src/transition.rs @@ -10,10 +10,11 @@ use reactive_graph::{ effect::Effect, owner::{provide_context, use_context, Owner}, signal::ArcRwSignal, - traits::{Get, Set, Track, With}, + traits::{Get, Set, Track, With, WithUntracked}, wrappers::write::SignalSetter, }; use slotmap::{DefaultKey, SlotMap}; +use std::sync::Arc; use tachys::reactive_graph::OwnedView; /// If any [`Resource`](crate::prelude::Resource) is read in the `children` of this @@ -104,14 +105,19 @@ where provide_context(SuspenseContext { tasks: tasks.clone(), }); - let none_pending = ArcMemo::new(move |prev: Option<&bool>| { - tasks.track(); - if prev.is_none() && starts_local { - false - } else { - tasks.with(SlotMap::is_empty) + let none_pending = ArcMemo::new({ + let tasks = tasks.clone(); + move |prev: Option<&bool>| { + tasks.track(); + if prev.is_none() && starts_local { + false + } else { + tasks.with(SlotMap::is_empty) + } } }); + let has_tasks = + Arc::new(move || !tasks.with_untracked(SlotMap::is_empty)); if let Some(set_pending) = set_pending { Effect::new_isomorphic({ let none_pending = none_pending.clone(); @@ -127,6 +133,7 @@ where fallback, children, error_boundary_parent, + has_tasks, }) }) } diff --git a/leptos_server/src/once_resource.rs b/leptos_server/src/once_resource.rs index 5f91dde34..89b70ba2c 100644 --- a/leptos_server/src/once_resource.rs +++ b/leptos_server/src/once_resource.rs @@ -15,7 +15,7 @@ use codee::{ Decoder, Encoder, }; use core::{fmt::Debug, marker::PhantomData}; -use futures::Future; +use futures::{Future, FutureExt}; use or_poisoned::OrPoisoned; use reactive_graph::{ computed::{ @@ -258,11 +258,17 @@ where if let Some(suspense_context) = use_context::() { if self.value.read().or_poisoned().is_none() { let handle = suspense_context.task_id(); - let ready = SpecialNonReactiveFuture::new(self.ready()); - reactive_graph::spawn(async move { - ready.await; - drop(handle); - }); + let mut ready = + Box::pin(SpecialNonReactiveFuture::new(self.ready())); + match ready.as_mut().now_or_never() { + Some(_) => drop(handle), + None => { + reactive_graph::spawn(async move { + ready.await; + drop(handle); + }); + } + } self.suspenses.write().or_poisoned().push(suspense_context); } } diff --git a/reactive_graph/src/computed/async_derived/arc_async_derived.rs b/reactive_graph/src/computed/async_derived/arc_async_derived.rs index c967fbc9e..46d703918 100644 --- a/reactive_graph/src/computed/async_derived/arc_async_derived.rs +++ b/reactive_graph/src/computed/async_derived/arc_async_derived.rs @@ -632,12 +632,29 @@ impl ReadUntracked for ArcAsyncDerived { fn try_read_untracked(&self) -> Option { if let Some(suspense_context) = use_context::() { + // create a handle to register it with suspense let handle = suspense_context.task_id(); - let ready = SpecialNonReactiveFuture::new(self.ready()); - crate::spawn(async move { - ready.await; - drop(handle); - }); + + // check if the task is *already* ready + let mut ready = + Box::pin(SpecialNonReactiveFuture::new(self.ready())); + match ready.as_mut().now_or_never() { + Some(_) => { + // if it's already ready, drop the handle immediately + // this will immediately notify the suspense context that it's complete + drop(handle); + } + None => { + // otherwise, spawn a task to wait for it to be ready, then drop the handle, + // which will notify the suspense + crate::spawn(async move { + ready.await; + drop(handle); + }); + } + } + + // register the suspense context with our list of them, to be notified later if this re-runs self.inner .write() .or_poisoned()