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

Apollo 2 Migration #2081

Closed
8 tasks done
SachaG opened this issue Sep 24, 2018 · 8 comments
Closed
8 tasks done

Apollo 2 Migration #2081

SachaG opened this issue Sep 24, 2018 · 8 comments
Labels

Comments

@SachaG
Copy link
Contributor

SachaG commented Sep 24, 2018

I'm opening a new thread to track my progress on the migration to Apollo 2.0 (starter repo).

Main Tasks

  • Apollo Client 2
    • Post-mutation data updates
    • Replace Redux with link-state
  • Apollo Server 2
  • React Router 4
    • Generate routes
    • Add callbacks, options, etc.
  • SSR
@SachaG
Copy link
Contributor Author

SachaG commented Sep 24, 2018

@SachaG
Copy link
Contributor Author

SachaG commented Sep 24, 2018

Here's a round-up of the relevant code that will need to be migrated/refactored/discarded:

vulcan:lib client

vulcan:lib modules

vulcan:lib server

vulcan:routing

See contents of https://github.com/VulcanJS/Vulcan/tree/devel/packages/vulcan-routing/lib Currently moved to vulcan:core/client/start.jsx.

@eric-burel
Copy link
Contributor

eric-burel commented Oct 5, 2018

Here is a summary of my latest commits. Globally, beware of bugs. The current branch should work fine, but I only tested it on the example-forms of the Starter package for the moment.

Server

  • It's (mostly) working 🎉
  • We added GraphQL Voyager on the /voyager endpoint thanks to @Apollinaire

But... there are a few things to handle, and I am not knowledgeable enough to fix them yet:

Other things to do:

  • Investigate using Data Sources instead of DataLoader to pass the collection to the resolvers. We agreed that keeping the current api (context.YourCollection) for now is a good idea. We can introduce Data Sources in a next version, while keeping the current syntax as a legacy for a while, there is no emergency. Also beware, the update might be trickier as it seems as there is caching involved.

Notes from my learning:

  • Using Meteor forces us to keep using apollo-sever-express, though we don't need to explicitely create an Express app (meteor/webapp does it for us). There are currently no example of a Meteor integration with a standalone Apollo server (so without express, using only the builtin HTTP server), and I am not sure it is actually doable.
  • Note that you now must separate server options computation from context computation (only the context option can take the current request and response into consideration, which was already true in v1 despite a misleading API).

RR4

RR4 works but I did not take a look at the second point (adding callbacks, options). See #2096 for example

Client

  • It (mostly) works 🎉, the withMessages hoc is a State Link usage example
  • Using State Link is very verbose, any input welcome to simplify syntax (we should especially try to rely on a 3rd party lib for this)

SSR

I did a test with using only an Express middleware, but this can only render dead HTML, as scripts are never injected by Meteor.
So I ended up using meteor/server-render onPageLoad callback, which behave mostly the same as an Express middleware. SSR is simply adding relevant HTML to the body of the response generated by Meteor. Then Apollo and React will handle the rehydratation.

  • create an Apollo client for the server (it fetches all the data it can during SSR). It relies on SchemaLink for the moment. Thus the server does not need to request himself using HTTP, it directly calls resolvers.

  • render the App as HTML server side.

  • populate the document using Helmet

  • inject the Apollo state

  • return the rendered HTML on requests
    Previously we relied on dynamicHead, that is then used by the template-web.browser.js file of _boilerplate-generator. Do we still need this? Instead I use meteor/server-render features to append content to the request body.
    This first set of features is enough to render a basic page like the getting-started first steps. Following features allow more complex usage, based on the example-forms app.

  • handle Apollo Link State server side. The stateLink API is now available client side and server side.

  • Define context of the SchemaLink, which is in turn passed to the resolvers and is necessary to actually get data.
    This context is then used in 2 places, as the context option of the server (using for each query), and as the context option of the SchemaLink (used during the initial SSR render). It is recomputed for every request.

  • Cookies. Could be improved though, as universal-cookies middleware does not seem to be taken in to account (see issue with meteor/server-render and meteor/webapp below.

  • Add auth with Cookie. On first page load, the Authorization header can't be set, so you can't authenticate. Solution is to rely on a cookie instead, and then only on the Authorization header when it is set. The cookie is updated on client startup based on the localStorage token. I uncommented existing code in auth.js and updated it, it seems fine.

  • Fix withSingle / SmartForm on edit SSR is broken #2111 (Form SSR/withSingle HOC is broken)

  • integrate advanced features of vulcan:routing (options, callbacks, etc.)? Some of them are now unnecessary, to be tested

  • handle errors. For the moment if the render fails we only display an error client side. In production you'd want the error to be caught and fallback to client side rendering, with maybe a console.warn (to be discussed). In dev mode you'd like the error to print nicely. Try for example to trigger an error in the default_resolvers, it will be hard to debug.
    I've added the ErrorLink server side to Apollo client but I don't know what it should do.

  • Check server side cache. Try the example-forms, fill a new Customer but do not add an address: you'll get crappy error messages about the cache (cache.readData is not defined) + error handling is broken. Sometimes the error is correct, sometimes it's weird.
    If you add an address it will correctly save the data however.

  • check for broken features, performance regression etc.

  • write. tests. many tests.

Less relevant stuffs:

Note: for the moment you should be able to disable SSR by commenting the line of apollo-server that triggers the enableSSR() function.

We will have to study thoroughly existing code in vulcan:routing (thanks @xavczen for the walkthrough!), as it handles many edge cases and provides options.

Backward compatibility and Vulcan-Starter

  • Created a vulcan-redux package. It allows legacy app to work by simply loading this package and imporing addAction, addReducer etc. directly from there. It allowed me to test the router.xxx.wrapper callbacks.
  • router.server.wrapper: allow to wrap the App component, for example to add a Redux store. Added to AppGenerator.jsx (untested though).
  • router.client.wrapper: same but client side
  • router.server.postRender (used to injectJSS for example): now takes the sink object + context as param instead of req/res. vulcan-material-ui will need a very small update to handle this breaking change, see https://docs.meteor.com/packages/server-render.html
  • Added a vulcan:styled-components independent package
  • Other router.server.preRender, router.server.html can be safely ignored, at least for the moment. They are not used in major packages/open source Vulcan example. wrapper and postRender already provides enough flexibility to add a complex lib such as Styled Components or Redux, including SSR.
  • vulcan:core/modules/callbacks: need to reset flash messages on route change
  • Deprecate Mongo_redux? It is used by the example-forms package, I guessed we can get rid of this.
  • Error404 handling in App.jsx

@SachaG
Copy link
Contributor Author

SachaG commented Oct 5, 2018

Using Meteor forces us to keep using apollo-sever-express, though we don't need to explicitely create an app. There are currently no example of a Meteor integration with a standalone Apollo server (so without express, using only the builtin HTTP server).

Maybe someone from the Meteor/Apollo team could help us? Or maybe @lorensr would know?

@lorensr
Copy link

lorensr commented Oct 5, 2018

Don't know! I had this question as well: apollographql/apollo-client#3739 (comment)

@lorensr
Copy link

lorensr commented Oct 5, 2018

@benjamn Is there a better way to set up Apollo Server inside Meteor? We're currently recommending https://www.apollographql.com/docs/react/recipes/meteor.html#Server

@eric-burel
Copy link
Contributor

eric-burel commented Oct 8, 2018

Hi,

Actually I think I figured this out wrong in the first place. It's not apollo-server-express that spawns the Express instance, it's just a connector. You always have to pass your own Express instance.
meteor/webapp does however setup a full fledged server (with connect to be precise, I guess this is mostly the same thing as Express? Not familiar with Node subtleties), with cookies, compression, etc., and your custom handlers. Code is here
So apolloServer.applyMiddleware({ app: Meteor.connectHandlers }) is roughly equivalent to apolloServer.applyMiddleware({app: yourExpressServer }). And thus you have to use apollo-server-express, since under the hood you are using Express when using meteor/webapp.

So my question itself does not make sense, you can't use the Apollo builtin server, at least not using the meteor/webapp package, because it's implementation rely on Express already. Anyway in Vulcan case it's a better idea to rely on Express due to 3rd party plugins compatibility (see graphql-voyager integration tested by @Apollinaire).

Edit:
To sum it up:

  • Meteor WebApp internally creates a Connect server (connect being the http framework used by Express itself that provides the middleware chaining pattern)
  • Therefore, Meteor does not seem to allow integration with HTTP servers others than Express and Connect. This include the Apollo 2 server. If there is a tighter integration between Meteor and Apollo, it won't be through meteor/webapp.
  • When writing const app = express(); app.use(...); WebApp.connextHandlers.use(app), you are actually chaining a new custom Express server to the WebApp Connect server. In most case this is redundant.
  • Use WebApp.connectHandlers.use(yourMiddeware) (eg for graphql-voyager etc.) or WebApp.connectHandlers.use(config.path, yourMiddleware) (eg for cookies, optimization etc.) instead of creating a new Express server.

@eric-burel
Copy link
Contributor

Closing in favor to #2171

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants