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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ export {
responseInterface,
CacheInterface
} from './types'
export { useSWRSuspense } from './use-swr-suspense'
export default useSWR
46 changes: 46 additions & 0 deletions src/use-swr-suspense.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// TODO: add documentation

import useSWR from './use-swr'

const suspenseGroup = {
promises: [],
started: false
}
Comment on lines +5 to +8
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.


function _internal_useSWRSuspenseStart() {
if (suspenseGroup.started) {
suspenseGroup.started = false
throw new Error('Wrong order of SWR suspense guards.')
}
suspenseGroup.started = true
suspenseGroup.promises = []
}

function _internal_useSWRSuspenseEnd() {
if (!suspenseGroup.started) {
throw new Error('Wrong order of SWR suspense guards.')
}
if (!suspenseGroup.promises.length) {
suspenseGroup.started = false
return
}
suspenseGroup.started = false
throw Promise.race(suspenseGroup.promises).then(() => {
// need to clean up the group
suspenseGroup.promises = []
})
}

function useSWRSuspense(callback) {
_internal_useSWRSuspenseStart()
const data = callback(useSWR)
pacocoursey marked this conversation as resolved.
Show resolved Hide resolved
_internal_useSWRSuspenseEnd()
return data
}

export {
suspenseGroup,
useSWRSuspense,
_internal_useSWRSuspenseStart,
_internal_useSWRSuspenseEnd
}
21 changes: 16 additions & 5 deletions src/use-swr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import defaultConfig, {
MUTATION_TS,
cache
} from './config'

import { suspenseGroup } from './use-swr-suspense'
import isDocumentVisible from './libs/is-document-visible'
import isOnline from './libs/is-online'
import throttle from './libs/throttle'
Expand Down Expand Up @@ -190,6 +192,9 @@ function useSWR<Data = any, Error = any>(
fn = config.fetcher
}

const inSuspenseGroup = suspenseGroup.started
const inSuspense = config.suspense || inSuspenseGroup

const initialData = cache.get(key) || config.initialData
const initialError = cache.get(keyErr)

Expand Down Expand Up @@ -560,7 +565,7 @@ function useSWR<Data = any, Error = any>(
])

// suspense
if (config.suspense) {
if (inSuspense) {
// in suspense mode, we can't return empty state
// (it should be suspended)

Expand All @@ -584,11 +589,17 @@ function useSWR<Data = any, Error = any>(
typeof CONCURRENT_PROMISES[key].then === 'function'
) {
// if it is a promise
throw CONCURRENT_PROMISES[key]
if (inSuspenseGroup) {
// we don't throw here if it's in suspense group
suspenseGroup.promises.push(CONCURRENT_PROMISES[key])
} else {
// single suspense swr
throw CONCURRENT_PROMISES[key]
}
} else {
// it's a value, return it directly (override)
latestData = CONCURRENT_PROMISES[key]
}

// it's a value, return it directly (override)
latestData = CONCURRENT_PROMISES[key]
}

if (typeof latestData === 'undefined' && latestError) {
Expand Down
42 changes: 41 additions & 1 deletion test/use-swr.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import {
} from '@testing-library/react'
import React, { ReactNode, Suspense, useEffect, useState } from 'react'

import useSWR, { mutate, SWRConfig, trigger, cache } from '../src'
import useSWR, {
mutate,
SWRConfig,
trigger,
cache,
useSWRSuspense
} from '../src'
import Cache from '../src/cache'

class ErrorBoundary extends React.Component<{ fallback: ReactNode }> {
Expand Down Expand Up @@ -1470,6 +1476,40 @@ describe('useSWR - suspense', () => {
expect(renderedResults).toEqual(['suspense-7', 'suspense-8'])
})

it('should work with suspense guards', async () => {
const fetcher = () => new Promise(res => setTimeout(() => res('data'), 100))

function Section() {
const [a, b, c] = useSWRSuspense(swr => {
const { data: a_ } = swr('suspense-guards-1', fetcher)
const { data: b_ } = swr('suspense-guards-2', fetcher)
// you can use `useSWR` too but the linter might yell
const { data: c_ } = useSWR('suspense-guards-3', fetcher)
return [a_, b_, c_]
})

// will be executed after *all* SWRs inside are resolved
expect(a).toBe('data')
expect(b).toBe('data')
expect(c).toBe('data')

return (
<div>
{a}, {b}, {c}
</div>
)
}
const { container } = render(
<Suspense fallback={<div>fallback</div>}>
<Section />
</Suspense>
)

expect(container.textContent).toMatchInlineSnapshot(`"fallback"`)
await act(() => new Promise(res => setTimeout(res, 110))) // it only takes 100ms
expect(container.textContent).toMatchInlineSnapshot(`"data, data, data"`)
})

it('should render initial data if set', async () => {
const fetcher = jest.fn(() => 'SWR')

Expand Down