Skip to content

WIP: feat: add suspense to useQueries #1754

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

Closed
wants to merge 1 commit into from
Closed
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
20 changes: 19 additions & 1 deletion src/core/queriesObserver.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import { difference, getQueryKeyHashFn, replaceAt } from './utils'
import { notifyManager } from './notifyManager'
import type { QueryObserverOptions, QueryObserverResult } from './types'
import type {
QueryObserverOptions,
QueryObserverResult,
RefetchOptions,
} from './types'
import type { QueryClient } from './queryClient'
import { QueryObserver } from './queryObserver'
import { Subscribable } from './subscribable'

type QueriesObserverListener = (result: QueryObserverResult[]) => void

interface QueriesRefetchOptions extends RefetchOptions {
filter: (observer: QueryObserver) => boolean
}

export class QueriesObserver extends Subscribable<QueriesObserverListener> {
private client: QueryClient
private result: QueryObserverResult[]
Expand Down Expand Up @@ -57,6 +65,16 @@ export class QueriesObserver extends Subscribable<QueriesObserverListener> {
return this.result
}

refetch(options?: QueriesRefetchOptions): Promise<QueryObserverResult[]> {
let observers = this.observers

if (options?.filter) {
observers = this.observers.filter(options.filter)
}

return Promise.all(observers.map(observer => observer.refetch(options)))
}

private updateObservers(): void {
let hasIndexChange = false

Expand Down
167 changes: 165 additions & 2 deletions src/react/tests/useQueries.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import { fireEvent, waitFor } from '@testing-library/react'
import React from 'react'
import { ErrorBoundary } from 'react-error-boundary'

import { queryKey, renderWithClient, sleep } from './utils'
import { useQueries, QueryClient, UseQueryResult, QueryCache } from '../..'
import { mockConsoleError, queryKey, renderWithClient, sleep } from './utils'
import {
useQueries,
QueryClient,
UseQueryResult,
QueryCache,
useQueryErrorResetBoundary,
} from '../..'

describe('useQueries', () => {
const queryCache = new QueryCache()
Expand Down Expand Up @@ -30,4 +38,159 @@ describe('useQueries', () => {
expect(results[1]).toMatchObject([{ data: 1 }, { data: undefined }])
expect(results[2]).toMatchObject([{ data: 1 }, { data: 2 }])
})

it('should render the correct amount of times in suspense mode', async () => {
const key1 = queryKey()
const key2 = queryKey()
const results: UseQueryResult[][] = []

let renders = 0
let count1 = 10
let count2 = 20

function Page() {
renders++

const [stateKey1, setStateKey1] = React.useState(key1)

const result = useQueries([
{
queryKey: stateKey1,
queryFn: async () => {
count1++
await sleep(10)
return count1
},
},
{
queryKey: key2,
queryFn: async () => {
count2++
await sleep(10)
return count2
},
suspense: true,
},
])

results.push(result)

return (
<button aria-label="toggle" onClick={() => setStateKey1(queryKey())} />
)
}

const rendered = renderWithClient(
queryClient,
<React.Suspense fallback="loading">
<Page />
</React.Suspense>
)

await sleep(50)

await waitFor(() => rendered.getByLabelText('toggle'))
fireEvent.click(rendered.getByLabelText('toggle'))

await sleep(50)

expect(renders).toBe(5)
expect(results.length).toBe(3)

// First load
expect(results[0]).toMatchObject([
{ data: 11, status: 'success' },
{ data: 21, status: 'success' },
])

// Set state
expect(results[1]).toMatchObject([
{ data: 11, status: 'success' },
{ data: 21, status: 'success' },
])

// Second load
expect(results[2]).toMatchObject([
{ data: 12, status: 'success' },
{ data: 21, status: 'success' },
])
})

it('should retry fetch if the reset error boundary has been reset with global hook', async () => {
const key1 = queryKey()
const key2 = queryKey()

let succeed = false
const consoleMock = mockConsoleError()

function Page() {
const results = useQueries<string>([
{
queryKey: key1,
queryFn: async () => {
await sleep(10)
if (!succeed) {
throw new Error('Suspense Error Bingo')
} else {
return 'data1'
}
},
retry: false,
suspense: true,
},
{
queryKey: key2,
queryFn: () => 'data2',
staleTime: Infinity,
},
])

return (
<div>
<div>data1: {results[0].data}</div>
<div>data2: {results[1].data}</div>
</div>
)
}

function App() {
const { reset } = useQueryErrorResetBoundary()
return (
<ErrorBoundary
onReset={reset}
fallbackRender={({ resetErrorBoundary }) => (
<div>
<div>error boundary</div>
<button
onClick={() => {
resetErrorBoundary()
}}
>
retry
</button>
</div>
)}
>
<React.Suspense fallback="Loading...">
<Page />
</React.Suspense>
</ErrorBoundary>
)
}

const rendered = renderWithClient(queryClient, <App />)

await waitFor(() => rendered.getByText('Loading...'))
await waitFor(() => rendered.getByText('error boundary'))
await waitFor(() => rendered.getByText('retry'))
fireEvent.click(rendered.getByText('retry'))
await waitFor(() => rendered.getByText('error boundary'))
await waitFor(() => rendered.getByText('retry'))
succeed = true
fireEvent.click(rendered.getByText('retry'))
await waitFor(() => rendered.getByText('data1: data1'))
await waitFor(() => rendered.getByText('data2: data2'))

consoleMock.mockRestore()
})
})
99 changes: 91 additions & 8 deletions src/react/useQueries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,114 @@ import React from 'react'
import { notifyManager } from '../core/notifyManager'
import { QueriesObserver } from '../core/queriesObserver'
import { useQueryClient } from './QueryClientProvider'
import { useQueryErrorResetBoundary } from './QueryErrorResetBoundary'
import { UseQueryOptions, UseQueryResult } from './types'

export function useQueries(queries: UseQueryOptions[]): UseQueryResult[] {
export function useQueries<TData = unknown, TError = unknown>(
Copy link

@johnnyreilly johnnyreilly Feb 13, 2021

Choose a reason for hiding this comment

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

Nice work!

I have a question. This adds the ability to specify generics for useQueries. Reading the code here, I think that this may fall into the "generic that is only used in the return position" anti-pattern category.

It's possible I'm reading it wrong so I wanted to ask a clarifying question: Does the approach here allow a user to specify types using the generic parameters that would differ from the return type of the query or of the select function?

If that's possible then the generics would not be adding a great deal of value as incorrectly typed data could still flow through.

Is that possible in this case?

As discussed, I'm generally keen on making use of type inference. But in the absence of that, avoiding type definitions that can possibly let incorrect types through is still worth aiming for.

queries: UseQueryOptions[]
): UseQueryResult<TData, TError>[] {
const queryClient = useQueryClient()
const errorResetBoundary = useQueryErrorResetBoundary()
const defaultedQueries: UseQueryOptions[] = []

let someSuspense = false
let someUseErrorBoundary = false

queries.forEach(options => {
const defaulted = queryClient.defaultQueryObserverOptions(options)

// Include callbacks in batch renders
if (defaulted.onError) {
defaulted.onError = notifyManager.batchCalls(defaulted.onError)
}

if (defaulted.onSuccess) {
defaulted.onSuccess = notifyManager.batchCalls(defaulted.onSuccess)
}

if (defaulted.onSettled) {
defaulted.onSettled = notifyManager.batchCalls(defaulted.onSettled)
}

if (defaulted.suspense) {
someSuspense = true
}

if (defaulted.useErrorBoundary) {
someUseErrorBoundary = true
}

defaultedQueries.push(defaulted)
})

if (someSuspense) {
defaultedQueries.forEach(options => {
// Always set stale time when using suspense to prevent
// fetching again when directly re-mounting after suspense
if (typeof options.staleTime !== 'number') {
options.staleTime = 1000
}

// Prevent retrying failed query if the error boundary has not been reset yet
if (!errorResetBoundary.isReset()) {
options.retryOnMount = false
}
})
}

// Create queries observer
const observerRef = React.useRef<QueriesObserver>()
const observer =
observerRef.current || new QueriesObserver(queryClient, queries)
observerRef.current || new QueriesObserver(queryClient, defaultedQueries)
observerRef.current = observer

// Update queries
if (observer.hasListeners()) {
observer.setQueries(queries)
observer.setQueries(defaultedQueries)
}

const [currentResult, setCurrentResult] = React.useState(() =>
observer.getCurrentResult()
)

let someError
let someIsLoading = false
let someIsError = false

currentResult.forEach(result => {
if (result.isLoading) {
someIsLoading = true
}

if (result.isError) {
someIsError = true
}

if (result.error) {
someError = result.error
}
})

// Subscribe to the observer
React.useEffect(
() => observer.subscribe(notifyManager.batchCalls(setCurrentResult)),
[observer]
)
React.useEffect(() => {
errorResetBoundary.clearReset()
return observer.subscribe(notifyManager.batchCalls(setCurrentResult))
}, [observer, errorResetBoundary])

// Handle suspense
if (someSuspense || someUseErrorBoundary) {
if (someSuspense && someIsLoading) {
errorResetBoundary.clearReset()
const unsubscribe = observer.subscribe()
throw observer
.refetch({ filter: x => x.getCurrentResult().isLoading })
.finally(unsubscribe)
}

if (someIsError) {
throw someError
}
}

return currentResult
return currentResult as UseQueryResult<TData, TError>[]
}