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

[RFC] useSWRSuspense (or Suspense guards) #168

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

shuding
Copy link
Member

@shuding shuding commented Dec 2, 2019

With useSWRSuspense you can avoid waterfall in suspense mode:

function App () {
  const [user, movies] = useSWRSuspense(()=> {
    const { data: user } = useSWR('/api/user')
    const { data: movies } = useSWR('/api/movies')
    return [user, movies]
  })

  // or
  // const [user, movies] = useSWRSuspense(swr => [swr('/api/user'), swr('/api/movies')])

  return (
    <div>
      Hi {user.name}, we have {movies.length} movies on the list.
    </div>
  )
}

Note that swr function is the exact same as useSWR, it's for avoiding linter errors

It will maximize the parallelism while under suspense mode. In the example above both APIs will start to fetch at the same time, and the actual render will be suspended until both are loaded.

Another proposal

Another way is to use 2 "Suspense guard" hooks:

function App () {
  useSWRSuspenseStart()
  const { data: user } = useSWR('/api/user')
  const { data: movies } = useSWR('/api/movies')
  useSWRSuspenseEnd()

  return (
    <div>
      Hi {user.name}, we have {movies.length} movies on the list.
    </div>
  )
}

Which does the same thing (but it might be easier for users to make mistakes).

Checklist

  • make sure it works in concurrent mode
  • better error handling
  • error if the callback throws a promise (misuse of suspense: true inside useSWRSuspense)
  • documentation

Fixes #5.

@pupudu
Copy link

pupudu commented Dec 2, 2019

Thank you for prioritizing this feature ❤️


In my head, the API should look something like below

function MyComponent(props) {
    // First we declare the list of fetch calls. Here we use an object. Could be an array instead.
    // Doing this will NOT trigger suspense. i.e. A promise shall not be thrown.
    // But the API calls will be triggered in parallel  
    const fetcher = useSuspenseSwr({
         key1: {/*fetch config for key 1*/},
         key2: {/*fetch config for key 2*/},
    });

    // and then call fetcher.fetch('key1') somewhere in the code
    // This is when the promise will be thrown
}

This way, the API calls are triggered immediately, and parts of the component which are not dependent on the API calls can render.


The problem I see in your API is that the fetch calls are completely blocking the render. It does avoid the waterfalls, but it does not give the full power of suspense to the users.

@samcx
Copy link
Member

samcx commented Dec 3, 2019

Thank you again for the amazing features and support!

If possible, I would write the API's like this:

export function App() {
  const [user, movies] = useSWR(
    ['/api/user', '/api/movies'], { suspense: true }
  );

// or const { user, movies } = useSWR(['/api/user', '/api/movies'], { suspense: true });
// or const [user, movies] = useSWR(swr => [swr('/api/user'), swr('/api/movies')], { suspense: true });

  return (
    <Suspense fallback={<Spinner />}>
      <div>
        Hi {user.name}, we have {movies.length} movies on the list.
      </div>
    </Suspense>
  );
}

Personally, I would prefer keeping the hook naming "clean" (sticking with just useSWR) by sticking with this commonly seen format ->

fetch(url: string, options: Object)

It would seem to make more sense to keep the suspense option in an options object, but I don't know what the consequences would be to the API. For example, what would the functionality be if a user were to have two fetches, but was missing the Suspense option? Do we just fetch two things before rendering like normal?

@sergiodxa
Copy link
Contributor

@samsisle what you proposed is somehow possible right now:

function fetcher(...urls) {
  return Promise.all(url.map(url => fetch(url).then(res => res.json())));
}

const { data } = useSWR(["/api/user", "/api/movies"], fetcher)
const [user, movie] = data;

Something like that, your fetcher could be technically anything and don't necessarily needs to do a single request so you could read your array of URLs and run multiples fetches.

Note this will mean if you change something in one of those URL (e.g. a param or query) all of them will be fetched again. The proposal useSWRSuspense should avoid that from what I saw, since it will let you still have multiple useSWR calls, one per URL.

@quietshu the second option is not so easy to understand, it's probably simple but I think I can get a sense on how the first one works just by seen the example, but with the second one I'm not sure.

@shuding
Copy link
Member Author

shuding commented Dec 3, 2019

@sergiodxa for the second example, it works like this under the hood:

useSWRSuspenseStart()                           // promises = []
const { data: user } = useSWR('/api/user')      // promises.push(fetch('/api/user'))
const { data: movies } = useSWR('/api/movies')  // promises.push(fetch('/api/movies'))
useSWRSuspenseEnd()                             // throw Promise.race(promises)

Comment on lines +5 to +8
const suspenseGroup = {
promises: [],
started: false
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen if I use useSWRSuspense or manually used _internal_useSWRSuspenseStart and _internal_useSWRSuspenseEnd in two component at the same time? Couldn't this cause some kind of race condition since both will modify this object at the same time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be fine because inside render,

useSWRSuspenseStart()
const { data: user } = useSWR('/api/user')
const { data: movies } = useSWR('/api/movies')
...
useSWRSuspenseEnd()    

This entire code block is atomic (synchronous). No other JavaScript code can be executed between useSWRSuspenseStart() and useSWRSuspenseEnd().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is still highly experimental. Currently I don't see anyone else in the community using this pattern. It needs more discussions and we are not planning to land it very soon.

@shuding
Copy link
Member Author

shuding commented Dec 3, 2019

The reason I'm not using

  const [user, movies] = useSWR(
    ['/api/user', '/api/movies'], { suspense: true }
  );

is because it loses the ability to do dependent fetching, meaning I can't do this in suspense mode:

image

(source)

But a function will still have the same expressiveness and parallelism (+ ability to be suspended by the boundary):

  const [user, movies] = useSWRSuspense(()=> {
    const { data: user } = useSWR('/api/user')
    const { data: posts } = useSWR('/api/posts')
    const { data: movies } = useSWR(() => '/api/movies?id=' + user.id)
    return [user, posts, movies]
  })

@shuding shuding added the feature request New feature or request label Dec 4, 2019
@shuding shuding marked this pull request as ready for review March 31, 2020 00:04
@shuding shuding changed the title [draft] useSWRSuspense (or Suspense guards) [RFC] useSWRSuspense (or Suspense guards) Mar 31, 2020
@Svish
Copy link
Contributor

Svish commented May 26, 2020

Couldn't this be done just by splitting things up?

  • If requests are not dependent on each other, they can be done in separate components.
  • If they are dependent, they can't be done in parallell anyways, and need to be done sequentially, which is how multiple useSWR( , { suspense: true }) works now, I believe?

@sergiodxa
Copy link
Contributor

@Svish I agree that ideally you should split your components and call it a day, however sometimes it's not possible to split things up.

I'm using a beta version of SWR with this feature because in a component I have multiple requests that are needed to render the UI, but they don't depend on each other, so they could totally be run simultaneously, without this feature I was causing an unnecessary waterfall of requests.

@shuding shuding added the RFC label Jun 9, 2020
@saitonakamura
Copy link

Can we ressurect this? I'm using swr for react-three-fiber (essentially vr application). In that ecosystem, suspense is heavily used, but waterfalling requests is a pretty much a show stopper for me. I'd be happy to help with testing/contributing

@saitonakamura
Copy link

saitonakamura commented Sep 20, 2021

So a feedback on the proposals: each of them has a certain flaw (well, they are already listed in the starting comment, but I think elaborating won't hurt)

  1. useSWRSuspense with a callback that calls useSWR - looks the best, flexible (existing hooks that wrap useSWR can be reused), but it's gonna produce a linter warning about a rules of hooks every time

  2. useSWRSuspense with a callback that uses swr - no linter error, but no reuse of existing wrapped useSWR hooks usage (or a refactoring work to allow them to accept it from the outside)

  3. useSWRSuspenceStart/End - Look kinda clunky, but the hooks can be reused and no linter errors

If you ask me, I would go for a third one, I favor flexibility and reusability and rely on linters heavily

@sergiodxa you said that you're using a beta that has this? Can you point where I can find this beta version if it still exists?

@shuding
Copy link
Member Author

shuding commented Sep 21, 2021

Thanks for the feedback @saitonakamura! Personally I like the third one as well, but I'd wait until Suspense & react-fetch (official data fetching lib with Suspense) get finalized first and then continue with this feature.

@sergiodxa
Copy link
Contributor

@saitonakamura I'm not using it anymore, the beta version for this is really old too, way before v1 of SWR and I don't remember the version to use in the package.json and I can't check in the project repository anymore.

The issues you mentioned are things I saw, linter complaining all the time, but I was able to use it custom hooks calling SWR internally

@saitonakamura
Copy link

@shuding waiting for suspense finalization makes sense (especially in vanilla react world). Although, if you're afraid of people adopting it, I guess it can be released under unstable_ prefix like react team does

@johanobergman
Copy link

What about this API?

function App() {
  // Start fetching the user data without suspending.
  const userResource = useSWRSuspense('/api/user')

  // Start fetching the movies data without suspending.
  const moviesResource = useSWRSuspense('/api/movies')

  // Throws the actual promise, suspends until the user data has loaded.
  const { data: user } = userResource.read()

  // Suspend until the movies data has loaded.
  const { data: movies } = moviesResource.read()

  return (...)
}

No hook lint errors, no waterfalls, data guaranteed to be available after the call to read().

If you want to do dependent fetching on more than one resource at the same time, only use suspense for the last hook call in the dependency chain.

@shuding
Copy link
Member Author

shuding commented Sep 26, 2021

@johanobergman It looks great! One limitation comparing to the original proposal is missing the ability to do "Promise.race", for example:

useSWRSuspenseStart()

const { data: foo } = useSWR('/foo')
const { data: bar } = useSWR('/bar')

// either `foo.id` or `bar.id` is ok here
const id = foo ? foo.id : bar ? bar.id : null
const { data: baz } = useSWR(id ? '/baz?id=' + id : null)

useSWRSuspenseEnd()

When the dependency gets more complicated, there is no easy way to be as parallelized as this API. The "preload + read" approach somehow still "defines" the order before blocking, while the wrapper solution works like a solver.

I'm not sure if this is critical in real-world applications though, but still a limitation.

@johanobergman
Copy link

@shuding Maybe make the api both suspense and not suspense at the same time?

function App() {
  // fooData and barData are only meant for dependent fetching, can be undefined.
  const { resource: fooResource, data: fooData } = useSWRSuspense('/foo')
  const { resource: barResource, data: barData }  = useSWRSuspense('/bar')

  const id = fooData ? fooData.id : barData ? barData.id : null
  const { resource: bazResource } = useSWRSuspense(id ? '/baz?id=' + id : null)

  // Every fetch is started, so we can block here.
  const { data: foo } = fooResource.read()
  const { data: bar } = barResource.read()
  const { data: baz } = bazResource.read()

  return (...)
}

@johanobergman
Copy link

Maybe there's no need for a useSwrSuspense hook at all, you could just include a "suspense handle" in every call to useSwr.

function App() {
  // Regular useSWR() calls, returns an optional suspense resource/handle as well as the data.
  const { resource: fooResource, data: fooData } = useSWR('/foo')
  const { resource: barResource, data: barData }  = useSWR('/bar')

  const id = fooData ? fooData.id : barData ? barData.id : null
  const { resource: bazResource } = useSWR(id ? '/baz?id=' + id : null)

  // Use read() to block, or check loading states as normal.
  const { data: foo } = fooResource.read()
  const { data: bar } = barResource.read()
  const { data: baz } = bazResource.read()

  // To make it (possibly) more explicit, you could read through a "blocker" function instead.
  const [foo, bar, baz] = suspenseBlocker([fooResource, barResource, bazResource])

  return (...)
}

@shuding
Copy link
Member Author

shuding commented Sep 26, 2021

Yeah that's a great idea too! 👍

@ConsoleTVs
Copy link

ConsoleTVs commented Mar 1, 2022

Althought probably pretty late to the party, this is the API that I ended up with when working on resolving the same issue on my vue port of SWR:

<script setup>
import { querySuspense } from 'vswr'

// Notice we don't use await here, and the result of those `querySuspense`
// are plain promises.
const first = querySuspense('/some/query') // Imagine this takes 2 seconds
const second = querySuspense('/some/other/query') // Imagine this takes 2 seconds

// This await will block until both resolve, but both already
// started fetching so the blockage is 2 seconds instead of
// 4 seconds if we awaited each one individually.
const [{ data: firstData }, { data: secondData }] = await Promise.all([first, second])
</script>

<template>
  <!-- Do something with firstData and secondData -->
</template>

More: https://github.com/ConsoleTVs/vswr#parallel-suspense

@mbrevda
Copy link

mbrevda commented Dec 26, 2022

is this PR... suspended?

@msdrigg
Copy link

msdrigg commented Mar 22, 2023

I am also curious about progress on this PR

@bayraak
Copy link

bayraak commented Jul 27, 2023

Any update on this?

@LarsFlieger
Copy link

@huozhi Do you know any updates on this? You seem to be active here :)

@huozhi huozhi marked this pull request as draft December 4, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request on hold RFC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoiding waterfalls with Suspense