-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
add vike-react-apollo #125
Conversation
🚀 Looking forward to merge 👌 You labeled it as draft so I guess it isn't ready for review yet, but let me know if you want a review. |
apollographql/apollo-client-nextjs#325 It would be nice to use the official |
Figured it out 😛 |
The PR "works fine" right now, but it's not perfect. The last chunk is not injected to the stream, because the stream is already closed. There are 3 ways to solve it - hardest to easiest:
|
If this is a reliable solution, then I'd be 👍 for doing this.
That could be nice as well 👍. Some kind of generic hook enabling the user to manipulate the stream.
Let's see if we need go for the bonus 😁 (or |
|
Review done. |
ready |
Reviewing done.
|
Idea: // +Loading.js
export default {
Layout: SomeLoadingComponent, // for both the Page and all Layouts
Component: SomeOtherLoadingComponent
}
// Sets Loading.Component
export default SomeOtherLoadingComponent WDYT? |
I like it because
🧐 |
The way I see it is that's it's similar to overload function arguments. setLoading(value) // What should this do?
setLoading({ component: value, layout: otherValue }) I'm inclined to make But maybe there is a better interface looming around the corner 🤔 |
Actually, I'm not sure: what should be the default page transition animation? I think we should eventually add one instead of just showing a blank page. I'm asking because maybe we want to make Or how about this: we only accept Unrelated: how about |
How about the "main Suspense approach" described at #117 (which I just updated)? So far I like this most. WDYT? |
done |
You're right, without |
LGTM. Feel free to merge & release. We need to manually squash the PR in two commits: one for |
No description provided.