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

Suspense only working with a single Resource #1805

Closed
flo-at opened this issue Sep 29, 2023 · 13 comments · Fixed by #1985
Closed

Suspense only working with a single Resource #1805

flo-at opened this issue Sep 29, 2023 · 13 comments · Fixed by #1985
Labels
bug Something isn't working

Comments

@flo-at
Copy link
Contributor

flo-at commented Sep 29, 2023

Describe the bug
After updating my code to reflect the breaking changes in 0.5.0-rc3, Suspense is not working anymore when there is more than one Resource involved.
It just stays in the suspense state, showing the fallback, forever.
In the browser's debug tag, I can see that the HTTP requests for both Resourcess returned successfully.
Also commenting out any of the two Resources I'm using makes it work again.
This is for a client-side rendered app and local Resources, created with create_local_resource.

Leptos Dependencies

leptos = { version = "0.5.0-rc3", features = ["csr", "nightly"] }                                   
leptos_meta = { version =  "0.5.0-rc3", features = ["csr", "nightly"] }                             
leptos_router = { version = "0.5.0-rc3", features = ["csr", "nightly"] }                            
leptos_reactive = { version = "0.5.0-rc3" }

To Reproduce
Steps to reproduce the behavior:
Use a Suspense component with more than on Resource in its children.

Expected behavior
According to the doc (and also working in pre 0.5 releases), Suspense should wait for all Resources in its children.

Additional context
In my case, both Resources are in the same child of Suspense. I'm note sure if that makes a difference. It used to work like that.

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

I just found #1751 while looking around. I'll try with master and see if it's fixed already.

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

Nope, it's still not working; tested with 609afce

@gbj
Copy link
Collaborator

gbj commented Sep 29, 2023

Please provide a reproducible example. I tried to build a reproduction and can't reproduce the issue.

use leptos::*;

async fn foo(inp: usize) -> String {
    gloo_timers::future::TimeoutFuture::new(250).await;
    format!("foo {inp}")
}

async fn bar(inp: usize) -> String {
    gloo_timers::future::TimeoutFuture::new(500).await;
    format!("bar {inp}")
}

#[component]
pub fn App() -> impl IntoView {
    let (start, set_start) = create_signal(0);
    let foo_r = create_local_resource(start, foo);
    let bar_r = create_local_resource(start, bar);
    view! {
        <button on:click=move |_| set_start.update(|n| *n += 1)>"+1"</button>
        <Suspense fallback=|| "Loading...">
            <p>{foo_r.get()}</p>
            <p>{bar_r.get()}</p>
        </Suspense>
        <Suspense fallback=|| "Loading...">
            <p>{foo_r.with(|n| n.clone())}</p>
            <p>{bar_r.with(|n| n.clone())}</p>
        </Suspense>
    }
}

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

Your example also works for me. I'll try to make another example that reproduces the problem.

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

This will reproduce the problem. If you remove any of the two resources from any of the children, it'll make that child work again (once you clicked the button, of course).

  use leptos::*;                                                                                      
                                                                                                      
  async fn foo(inp: usize) -> String {                                                                
      gloo_timers::future::TimeoutFuture::new(250).await;                                             
      format!("foo {inp}")                                                                            
  }                                                                                                   
                                                                                                      
  async fn bar(inp: usize) -> Option<String> {                                                        
      if inp > 0 {                                                                                    
          gloo_timers::future::TimeoutFuture::new(500).await;                                         
          Some(format!("bar {inp}"))                                                                  
      } else {                                                                                        
          None                                                                                        
      }                                                                                               
  }                                                                                                   
                                                                                                      
  #[component]                                                                                        
  pub fn App() -> impl IntoView {                                                                     
      let (start, set_start) = create_signal(0);                                                      
      let foo_r = create_local_resource(start, foo);                                                  
      let bar_r = create_local_resource(start, bar);                                                  
      view! {                                                                                         
          <button on:click=move |_| set_start.update(|n| *n += 1)>"+1"</button>                       
          <Suspense fallback=|| "Loading...">                                                         
              <p>{foo_r.get()}</p>                                                                    
              <p>{bar_r.get()}</p>                                                                    
          </Suspense>                                                                                 
          <Suspense fallback=|| "Loading...">                                                         
              <p>{foo_r.with(|n| n.clone())}</p>                                                      
A>            <p>{bar_r.with(|n| n.clone())}</p>                                                      
          </Suspense>                                                                                 
      }                                                                                               
  }

I guess this is because on the initial execution of bar, it returns None and doesn't await anything. The resource I have is depending on another resource, that's why I did it that way. Maybe that's not how one's supposed to do that.
I don't fully unterstand why it's stuck in the fallback state though.

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

One more thing: If you add gloo_timers::future::TimeoutFuture::new(0).await; the line before the None is returned, it will also work. The missing await on the initial execution seems to be the problem.

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

Just found out it's even simpler to reproduce using your example and leaving out (at least) one of the gloo_timers timeouts.

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

It's the same with Transition by the way. I hope that helps to narrow it down a bit.

@gbj gbj added bug Something isn't working and removed needs reproduction labels Sep 29, 2023
@gbj
Copy link
Collaborator

gbj commented Sep 29, 2023

Ok, so the issue is "if I have two resources, and one of them resolves immediately, Suspense is broken." I'll figure it out.

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

Thanks! That does fix the example indeed. My code still does not work unfortunately. I'll try to expand the example to get closer to what it does tomorrow.

@flo-at
Copy link
Contributor Author

flo-at commented Sep 29, 2023

This is closer to your initial example again and reproduces the problem for me:

  use leptos::*;                                                                                      
                                                                                                      
  async fn foo(inp: usize) -> String {                                                                
      gloo_timers::future::TimeoutFuture::new(250).await;                                             
      format!("foo {inp}")                                                                            
  }                                                                                                   
                                                                                                      
  async fn bar(inp: usize) -> String {                                                                
      gloo_timers::future::TimeoutFuture::new(500).await;                                             
      format!("bar {inp}")                                                                            
  }                                                                                                   
                                                                                                      
  #[component]                                                                                        
  pub fn App() -> impl IntoView {                                                                     
      let (start, set_start) = create_signal(0);                                                      
      let foo_r = create_local_resource(start, foo);                                                  
      let bar_r = create_local_resource(start, bar);                                                  
      view! {                                                                                         
          <button on:click=move |_| set_start.update(|n| *n += 1)>"+1"</button>                       
          <Suspense fallback=|| "Loading...">                                                         
          {move || {                                                                                  
              view! {                                                                                 
                  <p>{foo_r.get()}</p>                                                                
                  <p>{bar_r.get()}</p>                                                                
              }                                                                                       
            }                                                                                         
          }                                                                                           
          </Suspense>                                                                                 
          <Suspense fallback=|| "Loading...">                                                         
              <p>{foo_r.get()}</p>                                                                    
              <p>{bar_r.get()}</p>                                                                    
          </Suspense>                                                                                 
      }                                                                                               
  }

The one with the move closure doesn't work, the other does. It does not happen without the get():

<p>{foo_r}</p>                                                                
<p>{bar_r}</p>   

Still, it only happens when there is more than one resource in a child.
I know that in this case I could just remove all the get() calls. When I want to access the value to process it, I need the get() though.

@gbj gbj reopened this Oct 17, 2023
@gbj
Copy link
Collaborator

gbj commented Nov 3, 2023

@flo-at Apologies, I initially missed your additional example after I closed the issue. I think your last example is fundamentally the same issue as #1905, having to do with the fact that the two resources are read inside a single reactive closure, which means that when one of them changes, that closure re-runs, which reads the other resource again, incrementing the Suspense count an additional time that will never be decremented.

Note that the finer-grained version (in which only one Resource is read in a move || closure) does work:

<Suspense fallback=|| "Loading...">
  <p>{move || foo_r.get()}</p>
  <p>{move || bar_r.get()}</p>
</Suspense>

on nightly you can even just

<Suspense fallback=|| "Loading...">
  <p>{foo_r}</p>
  <p>{bar_r}</p>
</Suspense>

(The fact that that IntoView impl is missing on stable is my mistake and can be fixed)

It would be possible for me to fix this by changing the underlying Suspense implementation at the expense of some additional complexity there.

gbj added a commit that referenced this issue Nov 4, 2023
…closes `Suspense` only working with a single `Resource` (closes #1805, closes #1905)
@gbj gbj closed this as completed in #1985 Nov 4, 2023
gbj added a commit that referenced this issue Nov 4, 2023
…closes `Suspense` only working with a single `Resource` (closes #1805, closes #1905) (#1985)
@flo-at
Copy link
Contributor Author

flo-at commented Nov 4, 2023

Hi @gbj . Thanks for fixing this! I took a look into it some time ago but it seems to be quite complex. I also tried to rework my actual code into something that would work more like your finer-grained example but it's not so easy either. So the fix is much apprechiated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants