leptos icon indicating copy to clipboard operation
leptos copied to clipboard

Suspense does not work with conditionally accessed resources

Open haslersn opened this issue 1 year ago • 5 comments

Describe the bug

The following code accesses the resource hello and then, only if hello is loaded, accesses the resource world. Suspense does not properly work with this conditional access: Instead of rendering "Hello, World!", an empty page is rendered.

use leptos::*;
use leptos_meta::*;
use leptos_router::*;

#[component]
pub fn App() -> impl IntoView {
    provide_meta_context();

    view! {
        <Router>
            <Routes>
                <Route path="" view=Page/>
            </Routes>
        </Router>
    }
}

#[component]
fn Page() -> impl IntoView {
    let hello = create_resource(|| (), |_| delay_string("Hello,".to_string(), 2));
    let world = create_resource(|| (), |_| delay_string(" World!".to_string(), 4));

    view! {
        <Suspense fallback=|| {
            "Loading ..."
        }>
            {move || {
                let hello = hello.get();
                if hello.is_none() {
                    return None;
                }
                let world = world.get();
                if world.is_none() {
                    return None;
                }
                Some(
                    view! {
                        {hello}
                        {world}
                    },
                )
            }}

        </Suspense>
    }
}

#[server]
async fn delay_string(string: String, delay_in_secs: u64) -> Result<String, ServerFnError> {
    tokio::time::sleep(std::time::Duration::from_secs(delay_in_secs)).await;
    Ok(string)
}

Leptos Dependencies

leptos = { version = "0.6", features = ["nightly"] }
leptos_axum = { version = "0.6", optional = true }
leptos_meta = { version = "0.6", features = ["nightly"] }
leptos_router = { version = "0.6", features = ["nightly"] }

Expected behavior

I expect it to render "Loading ..." for 4 seconds and then "Hello, World!".

Actual behavior

It renders "Loading ..." for 2 seconds and then an empty page.

Probable reason:

  • The first time the Suspense renders its children, only the hello resource is accessed. Therefore the Suspense resolves after 2 seconds, when hello is loaded. At this point it renders its children again, which results in None, because the world resource is not yet loaded. But this time, the Suspense no longer applies the fallback and also doesn't watch for the world resource to load.

Additional context

I used a server function, because I couldn't call tokio::time::sleep on the client, but I think the issue has nothing to do with the server function.

haslersn avatar Feb 16 '24 17:02 haslersn

I agree that that will not work as you are hoping that it will work, based on the way that suspense is currently implemented.

I am going to mark this and #2308 as Feature request and 0.7. I am pretty sure that the changes to the way async stuff works in 0.7 mean that their equivalents there would already work as you're hoping. They cannot work in the current system.

For this one, it's worth noting that there's no benefit to not reading from world and doing the early return instead; both resources are already loading when created, so you're just not registering the second one with the suspense, which is causing the issue.

gbj avatar Feb 16 '24 18:02 gbj

it's worth noting that there's no benefit to not reading from world and doing the early return instead

In my case the fix was easy (access world earlier). However, there might be use cases where you have multiple world resources and, depending on the value of the hello resource, you decide which world resource you want to access (and register only that one with the Suspense).

haslersn avatar Feb 16 '24 18:02 haslersn

I've been hitting this case also. The issue comes from this code:

https://github.com/leptos-rs/leptos/blob/a2c7e23d54766e925a32effff0cdf8e27fd803e6/leptos_reactive/src/hydration.rs#L84-L89

Currently, the suspense component runs the childrenfn once via get_untracked(), then once the first resource resolves it will immediately jump into the code above and just render the suspense contents. What it should be doing instead is once all the pending resources resolved, it should run the scope again (it needs to do so anyway for the final SSR output) and only if there's still no pending resources, only then it should be moving to the final steps in generating and sending the output - otherwise it needs to repeat this process.

From a quick look at the source code it would seem that this problem would still persist in 0.7 (as the code for suspense_component and register_suspense doesn't look like it's been touched).

luxalpa avatar May 23 '24 17:05 luxalpa

From a quick look at the source code it would seem that this problem would still persist in 0.7 (as the code for suspense_component and register_suspense doesn't look like it's been touched).

leptos_reactive is essentially dead code in 0.7, where the Suspense system is totally reworked to use native .await on resources instead of the .get()-plus-null-checks system. I will leave this marked as 0.7 to make sure the scenario described above works as expected.

gbj avatar May 23 '24 18:05 gbj

That sounds interesting! Anyway I've been able to fix this bug for my local fork of leptos. I was about to make a pull request, but it would have contained a breaking change (signature change for register_suspense), and I guess if it's already resolved in .7, then there wouldn't be a way to merge the PR.

luxalpa avatar May 23 '24 19:05 luxalpa

Just tested this one out with the new async-based Suspend approach to Suspense and it works as expected and is a few lines shorter, without the null checks:

#[component]
fn HomePage() -> impl IntoView {
    let hello = Resource::new(|| (), |_| delay_string("Hello,".to_string(), 2));
    let world =
        Resource::new(|| (), |_| delay_string(" World!".to_string(), 4));

    view! {
        <Suspense fallback=|| {
            "Loading ..."
        }>
            {move || Suspend::new(async move {
                let hello = hello.await;
                let world = world.await;
                (hello, world) // same as view! { {hello} {world} }
            })}
        </Suspense>
    }
}

Note that while you'd ordinarily want to join() these two futures, you don't need to here — Resource parallelizes* the two server function calls and starts eagerly loading them, and then .await on the Resources themselves just waits for those to be ready, so this resolves in 4 seconds (because the two futures doing the actual work are running concurrently), not in 6 seconds (as if they were running serially).

* And I do mean "parallel" here: it will use tokio::spawn on the server so they may well end up running on different threads/cores in parallel, not only concurrently.

gbj avatar Aug 19 '24 11:08 gbj