Vulcan
Vulcan copied to clipboard
Apollo 2 - Improvements
Waiting for a fix in a 3rd party lib
Can't fix/improve those parts right now, would need improvements in Apollo, graphiql and GraphQL Playground.
- [X] Populate the
Authorization
header when using GraphQL PlayGround withlocalStorage['Meteor.loginToken
]. See https://github.com/prisma/graphql-playground/issues/510 for global Headers (right now we have to create an initial tab to setup headers) setup. The best solution seems to be using cookies. Also, in GraphiQL, we used thepassHeader
option that allowed access to the localStorage, but it was awfully hacky. See https://github.com/prisma/graphql-playground/issues/849 Note: Apollo 2 Chrome plugin provides a GraphiQL tab out of the box. Edit: maybe using cookies would be the solution? => it works if you are authenticated through the app and open graphQL only afterward, so it's ok - [ ] Change endpoint for GraphQL Playground. See https://github.com/apollographql/apollo-server/issues/1908 and https://github.com/apollographql/apollo-server/pull/1974 (pending PR so it should be implemented soon).
- [ ] Allow to override apollo server context (can be either an object or a function to compute context from req, so handling it is not trivial)
- [ ] Load GraphiQL as an official middleware. See https://github.com/graphql/express-graphql/issues/113. Have to use a hackish code inspired from https://github.com/eritikass/express-graphiql-middleware until there is an official alternative.
Possible improvements
- [ ] migrate away from Apollo Link State (not much used yet anyway) : https://www.apollographql.com/docs/react/essentials/local-state#migrating
- [ ] deprecate
webAppConnectHandlersUse
patch that changes middlewares order and makes/graphql
requests fail (but before that, check why we needed this patch in the first place) - [ ] create a callback to enhance the initial context
- [ ] create a callback to enhance the context on every request. Right now we have a
customContextFromReq
option (which is not really public), but that would make more sense to use a callback - [ ] handle SSR errors. I've added a try/catch around
renderToStringWithData
, which triggers the SSR render, but the error it produces are still a bit cryptic, you can't get which component is faulty - [ ] Investigate
apollo-link-state
asynchronicity. It seems to be async, even though it calls a local cache (so I would have expected to be sync), so the returned values can be undefined even when setting a default. SeewithMessages
for example:messages
can be undefined during loading. - [ ] Add safeguards to avoid this issue (making introspection queries async by accident, while they must be kept sync) https://github.com/apollographql/apollo-server/issues/1935
- [ ] Create a specific
vulcan:apollo
package? This would make it easier for future improvements. - [ ] Fix #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 - [ ] write. tests. many tests.
- [ ] 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. Edit: ErrorLink does not work server side (does not import), it is meant for client errors.
- [ ] 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. - [ ] 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). See
withMessages
for example. - [ ] 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. - [ ] WebApp (Express) middlewares can't seem to enhance the request object passed to the
meteor/server-render
onPageLoad
callback. The middleware runs correctly, but thesink.request
object is still the initial request. See https://github.com/meteor/meteor-feature-requests/issues/174#issuecomment-441047495 for more details (example with Cookies)
Done so far
- [X] Fixed
user
anduserId
in context. Now we usemeteor/apollo
getUser
, which encapsulate the logic that was need in v1 (checking the token and so on). I noticed that sometimes theauthorization
header is"null"
(as a String, not thenull
object`, because a JSON was stringified) though, this is not very clean, maybe we can improve the client part. - [x] Take a look at https://github.com/VulcanJS/Vulcan/issues/1662 and handle CORS correctly
- [X] Allow to configure the server. See for example PR #2147 (and the relevant code in Apollo2). See
registerApolloServerOptions
andregisterApolloApplyMiddlewareOptions
methods. - [X] 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. - [X] render the App as HTML server side.
- [x] Reenable
inject_data
. This allow to pass down data from the SSR rendering to the client, for example to fix #2218 by comparing the URL used during render and the client URL - [X] populate the document using Helmet
- [X] inject the Apollo state
- [X] return the rendered HTML on requests
Previously we relied on
dynamicHead
, that is then used by thetemplate-web.browser.js
file of_boilerplate-generator
. Do we still need this? Instead I usemeteor/server-render
features to append content to the request body. This first set of features is enough to render a basic page like thegetting-started
first steps. Following features allow more complex usage, based on theexample-forms
app. - [X] handle Apollo Link State server side. The stateLink API is now available client side and server side.
- [X] 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 thecontext
option of the SchemaLink (used during the initial SSR render). It is recomputed for every request. - [X] Cookies. Could be improved though, as
universal-cookies
middleware does not seem to be taken in to account (see issue withmeteor/server-render
andmeteor/webapp
below. - [X] 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. - [X] Created a
vulcan-redux
package. It allows legacy app to work by simply loading this package and imporingaddAction
,addReducer
etc. directly from there. It allowed me to test therouter.xxx.wrapper
callbacks. - [X]
router.server.wrapper
: allow to wrap the App component, for example to add a Redux store. Added toAppGenerator.jsx
(untested though). - [X]
router.client.wrapper
: same but client side - [X]
router.server.postRender
(used to injectJSS for example): now takes thesink
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 - [X] Added a
vulcan:styled-components
independent package - [X] 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
andpostRender
already provides enough flexibility to add a complex lib such as Styled Components or Redux, including SSR. - [X] Fix
meteor_login_token
cookie issue. The cookie must be set everytime the localStorage token is updated, seeauth.js
- [X] Allow to register custom links/terminating links (used by vulcan:files for uploading)
- [x] Allow disabling SSR (one the server it's a one liner, on the client we need to prevent rehydratation I think)
Pretty amazing. I feel like you've almost completely refactored Vulcan's back-end! Really excited to work with the new Apollo packages moving forward :)
Here's some notes on what I had to do to upgrade properly:
- update apollo-client
- update apollo-server-express
- update graphql
- update react-apollo
- update react-router
- update react-router-bootstrap
- update body-parser
- update express
- npm i graphql-voyager --save-dev
- npm i apollo-server
- npm i apollo-cache-inmemory
- npm i apollo-link-schema
- npm i apollo-link-state
- npm i apollo-link-error
- npm i react-router-dom
- npm i apollo-link-watched-mutation
- npm i apollo-link-http
React Router 4
- link is now imported from react-router-dom
- no more props.location.query
Issues
- tmeasday:check-npm-versions -> wrong version of
apollo
meteor package
A few additional notes for RR4 update:
-
IndexLink
no longer exists. Use<NavLink exact ... >
instead -
Link
andNavLink
are now imported fromreact-router-dom
(instead of react-router) - Only
NavLink
tolerate styling, eg withactiveClassName
prop.Link
are now simple<a>
tags as a default. -
onlyActiveOnIndex
can be replaced byexact
. -
this.props.router.isActive(path)
becomesmatchPath(this.props.location.pathname, { path })
(matchPath
is provided byreact-router
)
Some more issues I've noticed:
- [x] Event tracking is broken:
TypeError: getRenderContext is not a function
at trackInternal (internal-client.js:6)
at track (events.js:21)
- [x]
queryOne
/runGraphQL
is broken? (Error: Expected undefined to be a GraphQL schema
) - [ ] GraphQL errors don't appear in the terminal (only in devtools network tab)
- [x] i18n doesn't work for GraphQL content
- [ ] current user is not logged in after hot code reloading