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.

Sign up for GitHub

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

Jump to bottom

with-apollo SSR example added. #21956

Merged
merged 2 commits into from
Feb 9, 2021
Merged

with-apollo SSR example added. #21956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f93e962
with-apollo SSR example added.
0xtimsb Feb 8, 2021
8281452
Merge branch 'canary' into canary
leerob Feb 9, 2021
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
3 changes: 3 additions & 0 deletions examples/with-apollo/components/Header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ export default function Header() {
Client-Only
</a>
</Link>
<Link href="/ssr">
<a className={pathname === '/ssr' ? 'is-active' : ''}>SSR</a>
</Link>
<style jsx>{`
header {
margin-bottom: 25px;
Expand Down
33 changes: 33 additions & 0 deletions examples/with-apollo/pages/ssr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import App from '../components/App'
import InfoBox from '../components/InfoBox'
import Header from '../components/Header'
import Submit from '../components/Submit'
import PostList, {
ALL_POSTS_QUERY,
allPostsQueryVars,
} from '../components/PostList'
import { initializeApollo, addApolloState } from '../lib/apolloClient'

const SSRPage = () => (
<App>
<Header />
<InfoBox>ℹ️ This page shows how to use SSR with Apollo.</InfoBox>
<Submit />
<PostList />
</App>
)

export async function getServerSideProps() {
const apolloClient = initializeApollo()

await apolloClient.query({
query: ALL_POSTS_QUERY,
variables: allPostsQueryVars,
})

return addApolloState(apolloClient, {
props: {},
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't you want to render the posts and use the data you fetched?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, we are returning apolloClient, which contains fetched data, and it adds all the posts to the apollo cache, we have at a client.

Then, inside the <PostList /> component, that data is queried and the apollo client provides it from the cache.

We are using the same technique in the SSG example to use fetched data.

return addApollState(apolloClient, {
   props: {},
}

Copy link
Member

@leerob leerob Feb 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting. I know that's not your responsibility, because you're just following the pattern already set, but that seems confusing. All other usage of getStaticProps / getServerSideProps show forwarded the data to the component via props (hence the names). I wonder how many people are getting stuck on this.

Copy link

Choose a reason for hiding this comment

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

Interesting. I know that's not your responsibility, because you're just following the pattern already set, but that seems confusing. All other usage of getStaticProps / getServerSideProps show forwarded the data to the component via props (hence the names). I wonder how many people are getting stuck on this.

I've adopted the same convention as above following the examples available, but if there are some better practice / more straightforward I'd love to hear about them!

Copy link

Choose a reason for hiding this comment

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

We ran into this issue/decision on our project. Why isn't the best practice to just create an ApolloClient inside gSSP, fetch whatever and pass it as normal props to the page?

Copy link

Choose a reason for hiding this comment

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

That's the issue: you can't because functions are not JSONable, and what Next abstracts from us is basically a REST call (or something). But you don't have to right? fetchMore is only sugar for passing just a cursor (and Apollo will fill the rest of the vars with whatever was there before). If you hydrate on the first render on the client side, you should still be perfectly able to fetch the cursor from your fetched data and pass it to fetchMore. Really the only difference is that the first render will have cached Apollo Client data. This is the only connecting between Apollo and Next (with SSR). You fill the cache with whatever arbitrary data you want (most of the times, the first data the client will need). From the pages side, none of this matters, they just do whatever queries they need, and Apollo will have them in cache, they code stays the same. The only issue here (I think) is how to hydrate this state, and IMO it can be done with useApolloClient (get the client, hydrate data, etc etc), right before you useQuery. Of course you can abstract this away with a custom hook to reuse between pages. Something like useHydratedQuery.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see that makes sense. You are right, making a custom hook for this purpose would be really beneficial. But adding such complex logic inside with-apollo, wouldn't that be overkill for people who are just getting started. I think the current implementation is good, from the perspective of the example. We ain't creating a full-fledged app in here. Haha.

Copy link

Choose a reason for hiding this comment

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

Yeah that's also true. I mean if it works, it can't be that bad. But note that this current example (and the ones that existed before) doesn't point (IMO) in the best direction, which basically would be align with Next's way of thinking. With this said, it's not that complex, and I think anyone who is serious about Next will ask the same question we are asking and decide for themselves.

As for the hook, idk, it's as complex as what we currently have. Tbh I don't have a strong opinion to leave or remove, I'm just curious to see what core thinks (@leerob ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep same. Let's see.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that both approaches work. I just worry about beginners seeing it and getting confused.

Maybe it just needs a comment to explain why?

})
}

export default SSRPage