Skip to content

Commit

Permalink
fix: allow suspense lifecycle callbacks without using query instances
Browse files Browse the repository at this point in the history
  • Loading branch information
tannerlinsley committed May 12, 2020
1 parent efd8654 commit 65543e5
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 39 deletions.
21 changes: 11 additions & 10 deletions src/queryCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,6 @@ export function makeQueryCache() {
}

query.updateInstance = instance => {
// Filter out any placeholder instances created for suspense
query.instances = query.instances.filter(d => !d.isPlaceholder)

let found = query.instances.find(d => d.id === instance.id)

if (found) {
Expand All @@ -310,9 +307,7 @@ export function makeQueryCache() {

// Return the unsubscribe function
return () => {
query.instances = query.instances.filter(
d => !d.isPlaceholder && d.id !== instanceId
)
query.instances = query.instances.filter(d => d.id !== instanceId)

if (!query.instances.length) {
// Cancel any side-effects
Expand Down Expand Up @@ -406,6 +401,12 @@ export function makeQueryCache() {
// If there are any retries pending for this query, kill them
query.cancelled = null

const callbackInstances = [...query.instances]

if (query.wasSuspended) {
callbackInstances.unshift(query.suspenseInstance)
}

try {
// Set up the query refreshing state
dispatch({ type: actionFetch })
Expand All @@ -421,12 +422,12 @@ export function makeQueryCache() {
query.config.isDataEqual(old, data) ? old : data
)

query.instances.forEach(
callbackInstances.forEach(
instance =>
instance.onSuccess && instance.onSuccess(query.state.data)
)

query.instances.forEach(
callbackInstances.forEach(
instance =>
instance.onSettled && instance.onSettled(query.state.data, null)
)
Expand All @@ -444,11 +445,11 @@ export function makeQueryCache() {
delete query.promise

if (error !== query.cancelled) {
query.instances.forEach(
callbackInstances.forEach(
instance => instance.onError && instance.onError(error)
)

query.instances.forEach(
callbackInstances.forEach(
instance =>
instance.onSettled && instance.onSettled(undefined, error)
)
Expand Down
9 changes: 1 addition & 8 deletions src/tests/useQuery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,14 +337,7 @@ describe('useQuery', () => {

await waitForElement(() => rendered.getByText('todo aaaa'))

console.log(queryCache.queries)

// TODO: This passes in node 10 and 12 both locally and in CI,
// but fails in CI when using node 12.... Not sure what to do here.

// FYI, it thinks that it's equal to 2, not 1

// expect(Object.keys(queryCache.queries).length).toEqual(1)
expect(Object.keys(queryCache.queries).length).toEqual(1)
})

// See https://github.com/tannerlinsley/react-query/issues/160
Expand Down
36 changes: 15 additions & 21 deletions src/useBaseQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,30 +60,25 @@ export function useBaseQuery(queryKey, queryVariables, queryFn, config = {}) {
[query]
)

const updateInstance = React.useCallback(
isPlaceholder => {
query.updateInstance({
id: instanceId,
isPlaceholder,
onStateUpdate: () => rerender({}),
onSuccess: data => getLatestConfig().onSuccess(data),
onError: err => getLatestConfig().onError(err),
onSettled: (data, err) => getLatestConfig().onSettled(data, err),
})
},
[getLatestConfig, instanceId, query, rerender]
)

// Create the placeholder instance of this query (suspense needs this to
// to fire things like onSuccess/onError/onSettled)
updateInstance(true)
query.suspenseInstance = {
onSuccess: data => getLatestConfig().onSuccess(data),
onError: err => getLatestConfig().onError(err),
onSettled: (data, err) => getLatestConfig().onSettled(data, err),
}

// After mount, subscribe to the query
React.useEffect(() => {
// Update the instance to the query again, but not as a placeholder
updateInstance()
query.updateInstance({
id: instanceId,
onStateUpdate: () => rerender({}),
onSuccess: data => getLatestConfig().onSuccess(data),
onError: err => getLatestConfig().onError(err),
onSettled: (data, err) => getLatestConfig().onSettled(data, err),
})

return query.subscribe(instanceId)
}, [getLatestConfig, instanceId, query, rerender, updateInstance])
}, [getLatestConfig, instanceId, query, rerender])

React.useEffect(() => {
// Perform the initial fetch for this query if necessary
Expand All @@ -92,8 +87,7 @@ export function useBaseQuery(queryKey, queryVariables, queryFn, config = {}) {
!query.wasPrefetched && // Don't double fetch for prefetched queries
!query.wasSuspended && // Don't double fetch for suspense
query.state.isStale && // Only refetch if stale
(getLatestConfig().refetchOnMount ||
query.instances.filter(d => !d.isPlaceholder).length === 1)
(getLatestConfig().refetchOnMount || query.instances.length === 1)
) {
refetch().catch(Console.error)
}
Expand Down

0 comments on commit 65543e5

Please sign in to comment.