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

Minimal apollo example #387

Closed
nickredmark opened this issue Dec 13, 2016 · 60 comments
Closed

Minimal apollo example #387

nickredmark opened this issue Dec 13, 2016 · 60 comments
Assignees

Comments

@nickredmark
Copy link

nickredmark commented Dec 13, 2016

It turns out apollo integration is much easier when using apollo-client directly instead of react-apollo.

Here's the code: https://github.com/nmaro/apollo-next-example
And here's a running version (at least for as long as I keep the graphql server online): https://apollo-next-example-oslkzaynhp.now.sh

The relevant details are here:

apollo.js

import ApolloClient, {createNetworkInterface} from 'apollo-client'

export default new ApolloClient({
  networkInterface: createNetworkInterface({
    uri: GRAPHQL_URL
  })
})

then in a page

import React from 'react'
import gql from 'graphql-tag'
import 'isomorphic-fetch'
import apollo from '../apollo'
import Link from 'next/link'

const query = gql`query {
  posts {
    _id
  	title
  }
}`
export default class extends React.Component {
  static async getInitialProps({req}) {
    return await apollo.query({
      query,
    })
  }
  render() {
	...
  }
}
This was referenced Dec 13, 2016
@nickredmark
Copy link
Author

nickredmark commented Dec 13, 2016

Some observations: this approach doesn't go deep into the components to load all graphql queries it encounters (something you can enable server-side with react-apollo).

I believe this is a bit problematic with next.js: you aren't really meant to load data deep in the component hierarchy - if you want it to happen both on the client and on the server-side. There is just one point to load the data: in getInitialProps in the root component. I don't know if this is going to change in the future, but if not, then we will have to

  1. architecture our apps so that we load all relevant data for a page from the start, or
  2. a part of the data only on the client (namely everything we don't load in getInitialProps), with a different strategy

In both cases, the approach above should be fine for the data being loaded in getInitialProps.

@nickredmark
Copy link
Author

Aand if some core developer likes this, I can create a pull request with the example.

@sedubois
Copy link
Contributor

About getInitialProps only called at root, see #192. Would love to have your ideas there.

@amccloud
Copy link
Contributor

@sedubois what problems were you running into with react-apollo?

@sedubois
Copy link
Contributor

@nmaro your https://github.com/nmaro/apollo-next-example is empty.

@amccloud better to ask @nmaro about that (I still need to get back into the code).

@nickredmark
Copy link
Author

Thanks @sedubois it's now online (always forget to run push origin master instead of just push the first time).

@amccloud
Copy link
Contributor

Oops, I mentioned to the wrong person. @nmaro what issue did you have with react-apollo?

@nickredmark
Copy link
Author

The data was loaded in the server, then as soon as the client started loading the page was empty again. I then looked at @sedubois's implementation (https://github.com/RelateMind/relate), and thought it's already quite complex for a quick proof-of-concept, so I finally tried with the lower-level api.

@nickredmark
Copy link
Author

@stubailo since you were wondering why it's so hard to integrate apollo with next.js - it looks like the only place you can fetch data both on the client and on the server is at the page's root component inside an async function called getInitialProps. I think the usual way to integrate react-apollo would only be useful on the client side.

@stubailo
Copy link

Interesting - are there any other data integrations with Next.js? It looks like using Redux is also pretty hard based on the examples I've seen.

Most modern data systems have some sort of global cache (Redux, Apollo, Relay) so I feel like there needs to be some sort of facility in Next to enable this.

@adamsoffer
Copy link
Contributor

adamsoffer commented Dec 15, 2016

How can we make Next.js play nicer with modern data systems with a global cache (Redux, Apollo, Relay)? I feel like this should be a big priority for the next release. @stubailo @rauchg

@rauchg
Copy link
Member

rauchg commented Dec 15, 2016

Absolutely. We have a Redux example on the wiki, we need to create more like those :)

@rauchg
Copy link
Member

rauchg commented Dec 15, 2016

It's not something we have to do on a release basis btw. We can just write a wiki tutorial at any time.

@rauchg
Copy link
Member

rauchg commented Dec 15, 2016

Btw @nmaro that example looks really neat, thanks for contributing. We can take that as a base and expand it

@stubailo
Copy link

Oh, odd - I didn't realize the issues involved. @nmaro what is it about react-apollo that makes things hard? seems like you should be able to follow the redux example almost exactly but do new ApolloClient where this uses createStore, and use ApolloProvider instead of Provider.

I'd love to work with someone to make a minimal example. This is our "hello world" example for React, it would be great to have a port for Next.js: https://github.com/apollostack/frontpage-react-app

@adamsoffer
Copy link
Contributor

@stubailo I'd love to work with you on a minimal example. I've been using the universal apollo microframework, Saturn, for a couple projects and would love to port them over to Next.js + Apollo ultimately :)

@stubailo
Copy link

Nice - yeah just making minimal modifications to the frontpage app to make it run on next.js instead of create-react-app would be my preference. then we can list it on our home page as well!

@nickredmark
Copy link
Author

@stubailo

A small issue was that data was loaded and rendered on the server, only to be replaced with nothing when loading on the client - I guess I just don't know apollo and next enough to get it right. Using apollo-client directly I didn't have this issue.

What's more tricky for server-rendering is if you have queries deeper in the hierarchy. React doesn't have a way to render things asynchronously, i.e. wait for each component to be ready before rendering it. Which means a ssr framework either has to

  1. go through the whole component tree twice, once to load the data, and once to render it.
  2. provide an async entry point in the root - this is next.js approach with getInitialProps

Now the question is whether apollo has a way to detect all the data calls that will be needed to render a component tree, and do this all in one function call that can be provided to getInitialProps.

@adamsoffer
Copy link
Contributor

@stubailo Is there a solve for this? ^

@sedubois
Copy link
Contributor

@nmaro @ads1018 have you seen getDataFromTree? As used e.g in my example: https://github.com/RelateMind/relate/blob/master/hocs/apollo.js

@sedubois
Copy link
Contributor

BTW I wonder if things can be simplified now that #301 is merged. Haven't looked into that yet.

@adamsoffer
Copy link
Contributor

@sedubois I checked that out thanks for sharing! Yeah, I imagine your example using react-apollo can be simplified with the new programmatic API (#301) that was just merged into Master so that you don't have to wrap all page components with your own HOC. If you make any progress on that, please let me know! Would be cool to get a next.js example on the apollo homepage :)

@sedubois
Copy link
Contributor

NB @ads1018 #301 is about extracting common code with CommonsChunkPlugin, not Programmatic API. But yes programmatic API will definitely help as well, looking forward to have it released.

@adamsoffer
Copy link
Contributor

adamsoffer commented Dec 25, 2016

Has anyone had any luck getting react-apollo working with the new 2.0.0-beta.2 release?

@adamsoffer
Copy link
Contributor

adamsoffer commented Dec 28, 2016

@sedubois @stubailo I pushed up my attempt at next + react-apollo if you want to take a look. You can find it here: https://github.com/ads1018/frontpage-next-app

One issue I'm facing right now is that components are only getting rendered client side and not server side. Perhaps we can use react-apollo's getDataFromTree method inside server.js? Or maybe inside our own custom <document> ? Any suggestions / pull requests are welcome!

Would love to eventually include this hello world example inside the Next examples folder and the Apollo home page.

@rauchg
Copy link
Member

rauchg commented Dec 28, 2016

The only pre-requisite for server-rendering the data is that it's returned as an object in getInitialProps, no need for overrides.

@adamsoffer
Copy link
Contributor

Gotcha. I think this is a bit difficult with react-apollo because as @nmaro pointed out:

the question is whether apollo has a way to detect all the data calls that will be needed to render a component tree, and do this all in one function call that can be provided to getInitialProps.

@adamsoffer
Copy link
Contributor

adamsoffer commented Jan 7, 2017

I've got a working example of React Apollo and Next 😄 🚀 I hope many of you find it useful. You can check it out here: https://github.com/ads1018/next-apollo-example (I've also deployed a demo using Now.)

I ended up using a HOC inside my page called withData() which wraps the page with ApolloProvider. I was initially turned off by using providers on a per page basis as opposed to once inside a single file, but I was convinced by some really smart people that it's better for readability and scalability. I actually think withData(MyComponent) looks quite nice and provides good context to the reader (no pun intended) that a particular page fetches data.

Thanks @bs1180 and @rauchg for pointing me in the right direction. If you'd like to add a with-apollo example to the repo let me know and I can create a pull request.

@sedubois
Copy link
Contributor

sedubois commented Jan 7, 2017

Thanks @ads1018 😊 Compared to my example https://Relate.now.sh, does this example solve the issue of using Apollo in deeply nested components (avoiding the cascade of getInitialProps)? Maybe the example should showcase that as it's the main pain point. And I'm sure adding this to the examples folder would be very appreciated.

@adamsoffer
Copy link
Contributor

adamsoffer commented Jan 8, 2017

@sedubois I'm unable to reproduce the error you referenced in #192. I'm using Apollo inside nested components without any problems. If you pull down my example and are able to reproduce it would you let me know?

@sedubois
Copy link
Contributor

sedubois commented Jan 10, 2017

Thanks @ads1018, things work great with the fixes in adamsoffer/next-apollo-example#2 🎉. I updated my example as well: https://github.com/RelateNow/relate

@estrattonbailey
Copy link

Nice work, @ads1018 @sedubois! I've been following along on this and #192, I've also been investigating prefetching/async views using Apollo and vanilla React.

Have you noticed or do you anticipate any performance issues with running getDataFromTree before each page displays? Since technically, that method renders the whole tree recursively, and then when getInitialProps returns, React renders the tree again (albeit with data from the cache).

Real nice solution 👍 I think rendering twice is the only option to ensure all child data is cached, just curious what you guys think about performance.

@adamsoffer
Copy link
Contributor

Hey @estrattonbailey - I haven't noticed any performance issues and I don't anticipate any. In fact, it's super snappy! As for running getDataFromTree, I've wrapped that method call inside a conditional that checks if we're on the server so it only ever gets called when a user first loads the app and is bypassed on all subsequent route changes. You can play around with the demo if you want to check out the performance. Please let me know if you have any feedback!

@sedubois
Copy link
Contributor

@ads1018 some ideas for your example:

  • simplify initialState like this
  • separate middleware, store and reducer in files like this
  • simplify isServer to typeof window !== 'undefined', drop !!ctx.req
  • extract that IS_SERVER const to lib, no need to pass it around as param

@estrattonbailey
Copy link

@ads1018 Great to hear! Nice little demo.

What I meant to ask is: how well will this scale? Though I haven't used Next yet, as I understand it, Next calls getInitialProps on each route transition, if available on a page component i.e. pages/page.js. On a full scale app/website with hundreds of nodes and lots of data coming in I imagine that rendering twice on each route could contribute to some latency.

The project I'm working on is a large scale editorial site, so I hope to do some benchmarking of different approaches, including yours. Would love to discuss more on twitter if you'd like. Thanks for your work!

@adamsoffer
Copy link
Contributor

adamsoffer commented Jan 12, 2017

@estrattonbailey Gotcha. I imagine it will scale very well. For the initial page load, getInitialProps will execute on the server only. You're correct that getInitialProps will be executed again on the client but no data would get requested twice because getDataFromTree is wrapped inside a conditional that checks if we're on the server or not.

Side note - if you're concerned about initial page load time due to lots of components and data being requested on a page you can always tell apollo to intentionally skip specific queries during SSR and offload them to the client by passing ssr: false in the apollo query options.

I'll connect with you on twitter if you want to discuss further :)

@rauchg
Copy link
Member

rauchg commented Jan 12, 2017

You're correct that getInitialProps will be executed again on the client but no data would get requested twice because getDataFromTree is wrapped inside a conditional that checks if we're on the server or not.

Important to keep in mind getInitialProps gets executed on the client side only when transitioning with <Link>, not after the initial load

@sedubois
Copy link
Contributor

@ads1018 @estrattonbailey AFAIK there are still indeed 2 renders server-side on first page load: getDataFromTree gets executed and renders the whole tree internally, then the render is called again to construct the HTML response. Don't think if there's any way to avoid that, but I guess is still quite performant thanks to the network round-trips avoided by SSR.

I guess the performance is maximum when the GraphQL server is hosted on the same machine as the Next.js server, so you could always try that if you'd be concerned with performance (at this point, I prototype my app with Graphcool for the backend, while the Next.js is deployed with Now/Zeit World).

@adamsoffer
Copy link
Contributor

adamsoffer commented Jan 14, 2017

@sedubois @estrattonbailey Correct me if I'm wrong, but we are still only rendering once. getDataFromTree does not render the tree, it simply returns a promise which resolves when the data is ready in our Apollo Client store. At the point that the promise resolves, our Apollo Client store will be completely initialized and we can optionally render the tree if we want to pass the stringified results in the response of an HTTP request but we're not doing that in my example.

@sedubois
Copy link
Contributor

getDataFromTree does not render the tree

@ads1018 AFAIK and looking at the Apollo code, it does render the tree recursively (just to trigger the Apollo data-fetching). So that's 2 renders server-side on first page load.

@sedubois
Copy link
Contributor

But anyway, thanks to your demo, now we have a usable integration between Apollo and Next, and remaining questions about Apollo SSR performance don't have anything specific to Next.js anymore I think. I'd suggest to ask questions about that over there.

@tpreusse
Copy link
Contributor

@sedubois what is a render anyhow? I'd call it walking and shaking the tree. Seems to be optimized pretty well – suppressed setState and no reconciliation into DOM.

@helfer
Copy link

helfer commented Feb 3, 2017

@ads1018 nice example! Looks like it's been added to the Wiki here as well, so this issue could probably be closed?

cc @rauchg

@stubailo
Copy link

stubailo commented Feb 3, 2017

We should get a blog post about Apollo + Next.js on the Apollo blog!

@sedubois
Copy link
Contributor

sedubois commented Feb 3, 2017

@stubailo @ads1018's example is great 👏 For something bigger using same Apollo principles, can check my app: https://github.com/relatenow/relate

@adamsoffer
Copy link
Contributor

adamsoffer commented Feb 3, 2017

Thanks @helfer. I'm thrilled with how it came out. I feel like I discovered the holy grail of app development with Next.js + Apollo. I've been meaning to follow up with a blog post in an effort to spread the gospel but just haven't gotten around to it. @stubailo I'd be happy to collaborate on a piece on the Apollo medium publication :)

@adamsoffer
Copy link
Contributor

adamsoffer commented Feb 3, 2017

Huge shout out to @sedubois for helping out with the example and his sweet app. 😄

@theadactyl
Copy link

@ads1018 would love to get you on the blog. When you're ready to chat about it, ping me (thea) on Apollo Slack. :)

@timneutkens
Copy link
Member

@helfer You are totally right. I should do a new issue pass to see if issues can be closed 😄

@stubailo @theadactyl awesome idea ❤️

@morgs32
Copy link

morgs32 commented Sep 4, 2017

Anyone know of an issue/PR to watch - regarding requesting data twice server side? Just curious

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests