-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 - Improvements #2171
Labels
Comments
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:
React Router 4
Issues
|
A few additional notes for RR4 update:
|
Closed
Some more issues I've noticed:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Authorization
header when using GraphQL PlayGround withlocalStorage['Meteor.loginToken
]. See Global headers graphql/graphql-playground#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 Feature: Equivalent to GraphiQL passHeader graphql/graphql-playground#849Note: 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
Possible improvements
webAppConnectHandlersUse
patch that changes middlewares order and makes/graphql
requests fail (but before that, check why we needed this patch in the first place)customContextFromReq
option (which is not really public), but that would make more sense to use a callbackrenderToStringWithData
, which triggers the SSR render, but the error it produces are still a bit cryptic, you can't get which component is faultyapollo-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.vulcan:apollo
package? This would make it easier for future improvements.vulcan:routing
(options, callbacks, etc.)? Some of them are now unnecessary, to be testedI'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.
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.withMessages
for example.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.
meteor/server-render
onPageLoad
callback. The middleware runs correctly, but thesink.request
object is still the initial request. See Add additional details to the server-render sink object meteor/meteor-feature-requests#174 (comment) for more details (example with Cookies)Done so far
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.registerApolloServerOptions
andregisterApolloApplyMiddlewareOptions
methods.SchemaLink
for the moment. Thus the server does not need to request himself using HTTP, it directly calls resolvers.inject_data
. This allow to pass down data from the SSR rendering to the client, for example to fix Vulcan-using sites break when loaded inside archive.org #2218 by comparing the URL used during render and the client URLPreviously 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 the
getting-started
first steps. Following features allow more complex usage, based on theexample-forms
app.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.universal-cookies
middleware does not seem to be taken in to account (see issue withmeteor/server-render
andmeteor/webapp
below.auth.js
and updated it, it seems fine.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.router.server.wrapper
: allow to wrap the App component, for example to add a Redux store. Added toAppGenerator.jsx
(untested though).router.client.wrapper
: same but client siderouter.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.htmlvulcan:styled-components
independent packagerouter.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.meteor_login_token
cookie issue. The cookie must be set everytime the localStorage token is updated, seeauth.js
The text was updated successfully, but these errors were encountered: