-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix: no hydration when new promise comes in #8383
Open
juliusmarminge
wants to merge
21
commits into
TanStack:main
Choose a base branch
from
juliusmarminge:julius/hydrate-promise
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+162
−17
Open
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
050c9d5
add failing repro test
juliusmarminge 524f5ff
update assertinos
juliusmarminge cf37452
add logg
juliusmarminge d82cfb2
ehm - maybe fix?
juliusmarminge 6eaf6fc
rm -only
juliusmarminge 8c7ccf2
make example
juliusmarminge 4efa642
upd
juliusmarminge f32f6e3
ad debug logs
juliusmarminge 77362c4
more debugging
juliusmarminge 5fa1a33
push
juliusmarminge 7eec72d
maybe???
juliusmarminge edda7ba
rm log
juliusmarminge 85ee5bc
revert
juliusmarminge 4d9645b
Merge branch 'main' into julius/hydrate-promise
TkDodo a6d28f5
Merge branch 'main' into julius/hydrate-promise
TkDodo fb114b9
Merge branch 'main' into julius/hydrate-promise
juliusmarminge c5eccf5
fix: ?
TkDodo 6dbdf34
fix: check for pending status again
TkDodo 3c1b221
fix: clear serverQueryClient between "requests"
TkDodo c691765
Merge branch 'main' into julius/hydrate-promise
TkDodo b7d9755
chore: remove logs
TkDodo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
'use server' | ||
|
||
import { revalidatePath } from 'next/cache' | ||
import { countRef } from './make-query-client' | ||
|
||
export async function queryExampleAction() { | ||
await Promise.resolve() | ||
countRef.current++ | ||
revalidatePath('/', 'page') | ||
return undefined | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import { countRef } from '../make-query-client' | ||
|
||
export const GET = () => { | ||
return Response.json({ count: countRef.current }) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,39 @@ | ||
// In Next.js, this file would be called: app/providers.tsx | ||
'use client' | ||
import { QueryClientProvider } from '@tanstack/react-query' | ||
|
||
// Since QueryClientProvider relies on useContext under the hood, we have to put 'use client' on top | ||
import { QueryClientProvider, isServer } from '@tanstack/react-query' | ||
import { ReactQueryDevtools } from '@tanstack/react-query-devtools' | ||
import * as React from 'react' | ||
import type { QueryClient } from '@tanstack/react-query' | ||
import { makeQueryClient } from '@/app/make-query-client' | ||
|
||
let browserQueryClient: QueryClient | undefined = undefined | ||
|
||
function getQueryClient() { | ||
if (isServer) { | ||
// Server: always make a new query client | ||
return makeQueryClient() | ||
} else { | ||
// Browser: make a new query client if we don't already have one | ||
// This is very important, so we don't re-make a new client if React | ||
// suspends during the initial render. This may not be needed if we | ||
// have a suspense boundary BELOW the creation of the query client | ||
if (!browserQueryClient) browserQueryClient = makeQueryClient() | ||
return browserQueryClient | ||
} | ||
} | ||
|
||
export default function Providers({ children }: { children: React.ReactNode }) { | ||
const [queryClient] = React.useState(() => makeQueryClient()) | ||
// NOTE: Avoid useState when initializing the query client if you don't | ||
// have a suspense boundary between this and the code that may | ||
// suspend because React will throw away the client on the initial | ||
// render if it suspends and there is no boundary | ||
const queryClient = getQueryClient() | ||
|
||
return ( | ||
<QueryClientProvider client={queryClient}> | ||
{children} | ||
<ReactQueryDevtools /> | ||
{<ReactQueryDevtools />} | ||
</QueryClientProvider> | ||
) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might actually be working
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we prefetch a query without
await
in a page that is static or cached, either fully or partially, and then client navigate to it with newer data in the cache? My guess is that there are/can be situations where we get a promise that will resolve to data that is actually older than the one we have on the client already?Maybe the entirely correct way to do this would be to inspect the data the query resolves to before determining whether to update the cache with that data? Implementation-wise that's a lot tricker though unfortunately. :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is something only users could verify though. Without query meta-data like
dataUpdatedAt
, react-query doesn’t know the age of the data. Are you suggesting we provide a way for users to extract a timestamp (age) from the promise data in some sort of callback option?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it? If we are using the fetch timestamp from the server to determine this when hydrating data, we can surely do that for promises too, just after they resolved? I'm sure it might be a big painful thing to implement, but is there some inherent thing about user/library land that blocks us from doing this in the library using the same logic?
To be clear, if we already have this query in the cache, this might require us to wait for the promise outside of the cache and only put the data in. Or even worse, to support fetching states properly, it might requires us to have some sort of "possibleUpdatePromise" or something that would not commit it's result to the cache if it's older.