Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource::loading causes orphaned elements when used for conditional rendering. #1457

Closed
mforsb opened this issue Jul 28, 2023 · 3 comments
Closed
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@mforsb
Copy link
Contributor

mforsb commented Jul 28, 2023

Describe the Bug
See title and expected/actual behavior. It seems to work fine when used to conditionally apply a class though.

Leptos Dependencies

leptos = { git = "https://github.com/leptos-rs/leptos", features = ["nightly"] }
leptos_axum = { git = "https://github.com/leptos-rs/leptos", optional = true }
leptos_meta = { git = "https://github.com/leptos-rs/leptos", features = ["nightly"] }
leptos_router = { git = "https://github.com/leptos-rs/leptos", features = ["nightly"] }

To Reproduce

cargo leptos new --git leptos-rs/start-axum --name bug
cd bug
cargo add serde --features derive
cargo add tokio --features time

change HomePage in src/app.rs to:

#[component]
fn HomePage(cx: Scope) -> impl IntoView {
    let resource = create_resource(cx, || (), |()| async { sleep().await });
    let loading = resource.loading();

    view! { cx,
        <button on:click=move |_| resource.refetch()>"Click Me"</button>
        {move || loading().then(move || view! { cx, <div>"Loading..."</div> })}
    }
}

#[server(Sleep, "/api")]
pub async fn sleep() -> Result<(), ServerFnError> {
    tokio::time::sleep(tokio::time::Duration::from_millis(1000)).await;
    Ok(())
}

Expected behavior

  • Loading... is visible only when the resource is loading.

Actual behavior

  • There is an extra Loading... permanently visible in addition to the one that appears when you press the button.
  • component with id _0-16c not found, ignoring it for hydration is printed in the console.
@gbj
Copy link
Collaborator

gbj commented Jul 29, 2023

This is essentially a hydration mismatch in a section of the view that creates an element depending on a resource without using <Suspense/>. If you were reading from the resource itself you'd get a nice runtime warning about this.

It's pretty straightforward to understand why it happens:

  1. your view renders on the server while loading, meaning it shows that loading message element
  2. resource resolves from the server to the browser
  3. by the time the hydration happens the resource has already loaded and is no longer loading
  4. but the Unit/None value isn't found in the DOM (hence the warning) and there's an extra unexpected element that's never hydrated and so is never removed

There are a couple possible solutions here:

  1. Use a <Suspense/>, which provided exactly this loading functionality and enables correct server rendering and hydration
  2. Use your custom loading().then() but wrapped in <Suspense/> so that the new HTML fragment ships correctly
  3. Make changes to the loading() signal so that if it's read outside Suspense in hydrate mode it initially returns true and then immediately false, allowing correct hydration at the cost of some additional runtime overhead.
  4. Rewrite the renderer to handle hydration differently

@gbj gbj added documentation Improvements or additions to documentation enhancement New feature or request and removed documentation Improvements or additions to documentation labels Jul 29, 2023
@mforsb
Copy link
Contributor Author

mforsb commented Jul 29, 2023

Thanks! That makes sense. I ended up using this:

#[component]
fn WhileLoading<S, T>(cx: Scope, resource: Resource<S, T>, children: ChildrenFn) -> impl IntoView
where
    S: Clone + 'static,
    T: Clone + 'static,
{
    view! { cx,
        <Suspense fallback=move || children(cx)>
            {move || resource.with(cx, |_| ())}
        </Suspense>
    }
}

which I presume is what you meant with 1.

I don't know if you want to deal with it on the leptos side or not so feel free to close if you want to.

@gbj gbj added the documentation Improvements or additions to documentation label Aug 23, 2023
@gbj
Copy link
Collaborator

gbj commented Aug 25, 2023

#1586 implements my third suggestion above

Make changes to the loading() signal so that if it's read outside Suspense in hydrate mode it initially returns true and then immediately false, allowing correct hydration at the cost of some additional runtime overhead.

I've tested it and this behaves correctly if you use loading() for conditional rendering either inside or outside a Suspense, so I'd consider this one closed on main and future releases.

@gbj gbj closed this as completed Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants