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

Adds support for useBackgroundQuery and useReadQuery in Streaming SSR #38

Merged
merged 23 commits into from
Jun 23, 2023

Conversation

alessbell
Copy link
Member

This PR creates a new class, NextSSRApolloClient in package/src/ssr/NextSSRApolloClient.tsx, which can be imported and used in client components where one would use ApolloClient outside of the app directory.

This extension of the ApolloClient class registers "background queries" (queries triggered by components rendering on the server) via the same registerLateInitializingQueue strategy already used to register results on the client. By keeping track of queries already run on the server, we can avoid duplicating these requests on the client when the same components are executed in the browser, since the result will be streamed in once it has completed.

@alessbell alessbell requested a review from phryneas June 14, 2023 16:04
@@ -1,5 +0,0 @@
import { PollSkeleton } from "@/components/poll";
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add this back - I temporarily removed it for testing with useBackgroundQuery where using a Suspense boundary directly was preferred

Copy link
Member

Choose a reason for hiding this comment

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

Reminder to add this back :)

Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

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

Already some feedback. I'm curious why that deduplication change didn't do what we expected, though. Gonna look a little deeper 👀

examples/polls-demo/app/cc/apollo-wrapper.tsx Outdated Show resolved Hide resolved
examples/polls-demo/app/cc/poll-cc.tsx Outdated Show resolved Hide resolved
package/README.md Show resolved Hide resolved
package/src/ssr/NextSSRApolloClient.tsx Outdated Show resolved Hide resolved
package/src/ssr/NextSSRApolloClient.tsx Outdated Show resolved Hide resolved
package/src/ssr/NextSSRApolloClient.tsx Outdated Show resolved Hide resolved
package/src/ssr/NextSSRInMemoryCache.tsx Show resolved Hide resolved
@phryneas
Copy link
Member

phryneas commented Jun 16, 2023

@alessbell I found out what was going wrong.

For type safety, we need a getQuerymanager(this) method returning a typed QueryManager<TCacheShape> instance instead of just accessing this['QueryManager'] everywhere, as that one is any, not QueryManager<TCacheShape>.

Add that and you'll immediately notice ;)

@alessbell
Copy link
Member Author

Thanks for the review, @phryneas! I believe I've gotten to all the feedback (including offline comment re: test helpers addressed in 037c8ba) and I've done some more manual testing. This is ready for a final look :)

@alessbell alessbell requested a review from phryneas June 21, 2023 20:57
@phryneas phryneas mentioned this pull request Jun 22, 2023
Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

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

This looks very good!
Only some minor little changes, then this should be good to go!

package/src/ssr/hooks.ts Outdated Show resolved Hide resolved
package/src/ssr/NextSSRApolloClient.tsx Outdated Show resolved Hide resolved
package/src/ssr/NextSSRApolloClient.tsx Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants