feat: suspense guard in data and error getters

[h3rmanj]

Alternative to #168 Fixes #5

This PR isolates the suspense guarding into a function, which is then called in the data and error getters returned from useSWR.

This allows you to write your suspense useSWRs like this to avoid waterfalls;

function App () {
  const user = useSWR('/api/user')
  const movies = useSWR('/api/movies')

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

// OR

function App () {
  const userRequest = useSWR('/api/user')
  const moviesRequest = useSWR('/api/movies')
  const user = userRequest.data
  const movies = moviesRequest.data

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

This also allows for dependency fetching

const user = useSWR('/api/user')
const posts = useSWR('/api/posts')
// Accessing .data will throw during key evaluation, letting swr know it's not ready
const movies = useSWR(() => '/api/movies?id=' + user.data.id)

And it would also (mostly) be backwards compatible, since most usage uses deconstruction which will automatically call the data and/or error getters, preserving earlier functionality.

I believe this is a much cleaner interface than the other ones proposed. I've yet to test this completely, so it's mostly a proposal for now, feedback needed 😄

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 99b7b73e618636f4d5e422007c36722ea436e702:

Sandbox	Source
SWR-Basic	Configuration
SWR-States	Configuration
SWR-Infinite	Configuration
SWR-SSR	Configuration

[h3rmanj]

Codesandbox demo: https://codesandbox.io/s/swr-basic-forked-ogpwsw?file=/src/App.js

[h3rmanj]

Added some tests. Didn't test for error yet, as I'm not sure if it should throw at all if just reading errors, as it already does when reading data.

I'm not sure if we're going to go with this approach for suspense, since suspense is still a ongoing discussion. And this seems to change the existing behavior of suspense?

My goal with this PR was to add to the discussion with a specific implementation, which I believe to be much cleaner and more resembling of the official recommendations for Suspense (where you do a specific .read() at a point later than declaring your data dependency)

It does change the implementation in cases where consumers read from the .data property and not using object deconstructing to read the data property returned from useSWR. In my opinion, while this does change the implementations somewhat, in most cases the behavior wouldn't differ (using object deconstructing), and in the cases where consumers use suspense without object deconstructing (reading .data later), it would probably be an improvement (unless you somehow depend on a waterfall).

Either way this would fit in with the 2.0 release as well, on the possibility that it breaks for some consumers. Worth noting though that this didn't fail any existing tests.

I'm sorry if the PR was unwarranted, and I can write a summary of my proposal in the original RFC (which seemed somewhat abandoned) if desired 🙂

[nstepien]

So even something like obj.data === undefined would trigger suspense? We'd have to be very careful in cases where we don't want to suspend the page while it's fetching, IMO it seems less ergonomic than .read().

[h3rmanj]

Suspense would still need to be explicitly enabled in the options. This PR only changes when it's thrown, if the option is enabled.

[nstepien]

Ah I didn't notice that suspense: true is still needed (it's missing in the opening comment).

I think I'd still prefer .read():

• the suspense setting wouldn't be needed anymore
• .data type would still be T | undefined
• .read() type would always be T

In our codebase we wrap swr in custom hooks like this example:

interface MyData {
  readonly id: number;
}

const key = 'key';

function useData() {
  return useSWR<MyData>(key);
}

function useDataWithSuspense() {
  const { data, ...rest } = useSWR<MyData>(key, { suspense: true });
  return { data: data!, ...rest };
}

function useDataMaybeWithSuspense(suspense = true) {
  return useSWR<MyData>(key, { suspense });
}

Because the suspense setting doesn't change the return type we always have to add type assertions when using suspense: true. In case when suspense is controlled by the component like in useDataMaybeWithSuspense we have to be more careful about types.

A solution like .read() would be much more ergonomic for us, we'd only need the useData() above without having to worry about types.

Just my 2 cents.

[h3rmanj]

Yep, I agree having a read function returned would be more explicit and probably self-documenting. I wanted to explore this option as the properties returned from useSWR already are getter functions, and would be a minimal change for a much-needed feature.

You could technically do pattern matching with the types

type SWRSuspenseHook<T> = (suspense: true) => { data: T }
type SWRNormalHook<T> = (suspense?: false) => { data?: T }

type SWRHook<T> = SWRSuspenseHook<T> | SWRNormalHook<T>

but good luck doing that with the complex types of the existing SWRHook type 😄