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

[WIP] Apollo 2: apollo-state-link and RR4 #2083

Merged
merged 9 commits into from
Sep 27, 2018

Conversation

eric-burel
Copy link
Contributor

@eric-burel eric-burel commented Sep 25, 2018

Starter PR
I started to work on the replacement of redux by apollo-state-link.

So far I have:

  • read code

  • understood the role of redux related to apollo-client

  • learnt what are apollo links and how apollo-state-link is a replacement to redux in regards to data storing

  • moved files a little. Most of the code realted to apollo-client seems totally generic and reusable for any frontend client that would like to connect to the Vulcan app, so it sounds a good idea to isolate it.

  • wrote a very basic setup for apollo state link that does nothing

  • implemented registerMutation, registerDefault and deferred the Apollo client creation until startup.

  • wrote a few test but couldn't run them yet

  • wrapped default_mutations call to registerWatchedMutation with Meteor.isClient (apollo client methods have no meaning server side, so the code is now in client instead of modules)

  • updated core components to use React Router 4

  • fixed App.jsx route Switch exact props

Please correct me if I am wrong:

  • watchedMutation allows to update the local cache when the user create/update/delete data, so that data lists are updated too.
  • watchedMutation also helps to have a pseudo realtime features (when user A call createTodo, the watchedMutation of user B is called a few second later), or is this totally unrelated?
  • what we need now is a way to enhance the state link with new resolvers and defaults, the same as we were able to reload the redux store when adding new reducers/middlewares
  • then we will need to update the components that dispatched actions to the store, so that now they update the cache. Instead of defining actions, we now define cacheMutations that uses writeCache.
  • then update components thatused to read the redux store, so that they read the apollo client cache instead

@eric-burel eric-burel changed the title [WIP] apollo-state-link [WIP] Apollo 2: apollo-state-link and RR4 Sep 26, 2018
Eric Burel added 2 commits September 26, 2018 11:34
meteor/apollo Atmosphere version does not seem up to date with the Github repo
@SachaG
Copy link
Contributor

SachaG commented Sep 26, 2018

Nice!

watchedMutation allows to update the local cache when the user create/update/delete data, so that data lists are updated too.

Yep. It's a replacement for the old reducer or the new update property which is the "normal" way to do it in Apollo v2 (but which is not generic enough to work for us).

watchedMutation also helps to have a pseudo realtime features (when user A call createTodo, the watchedMutation of user B is called a few second later), or is this totally unrelated?

No I don't think it has anything to do with realtime.

what we need now is a way to enhance the state link with new resolvers and defaults, the same as we were able to reload the redux store when adding new reducers/middlewares

Yep.

then we will need to update the components that dispatched actions to the store, so that now they update the cache. Instead of defining actions, we now define cacheMutations that uses writeCache.

In core I think only withMessages depended on Redux?

then update components that used to read the redux store, so that they read the apollo client cache instead

Hopefully we can keep the same API.

@eric-burel
Copy link
Contributor Author

eric-burel commented Sep 26, 2018

I setup a demo code, but did not test it yet as for the moment no component seem to use addReducer in the core. In the starter, only the example forum uses it.

My current understanding:

  • addMutation, in the state link code, is a replacement to both addAction and addReducer. There does not seem to be a concept of middleware anymore. Instead you should be able to add a new Link, that will be applied to all request (not implemented in my example but the pattern is always the same).
  • You can access those mutations using the graphql HOC, as you would use the Redux mapDispatchToProps.
  • You can query data as usual in GraphQL, and even ask for client only data

The difficulty here is to be able to replace/add a mutation inside the state link, as replaceReducers does in Redux, which does not seem possible at the moment. My idea is to totally replace the store while keeping the cache but then how does the UI updates? Is destroying and recreating the client even possible without losing its state? Should it subscribe to updates of the apollo-client? This will need some experiments I think, however I lack the knowledge to setup realistic examples.

See the issue I opened on apollo-link-state:
apollographql/apollo-link-state#306

@SachaG
Copy link
Contributor

SachaG commented Sep 26, 2018

The difficulty here is to be able to replace/add a mutation inside the state link, as replaceReducers does in Redux, which does not seem possible at the moment.

We could probably work around that by adding our own registerXYZ method that registers a mutation with Vulcan but not with Apollo, and then initialize everything on startup? Kinda like we do for components, fragments, etc.

@eric-burel
Copy link
Contributor Author

Definitely what I thought afterward, I did not remember initialization was handled in startup e.g for fragments. We indeed will have to do the same for the apollo client, until they add some function like a "replaceLinks".

I am cleaning up the code, then I will merge to the apollo2 branch so we are up to date.
I will explore a bit more apollo in my own apps before coming back to updating Vulcan.

@eric-burel eric-burel merged commit f887153 into VulcanJS:apollo2 Sep 27, 2018
@eric-burel
Copy link
Contributor Author

I merged since I have a working version. I did not experiment the registerMutation and registerDefault, but on the paper they should work as expected, as long as they are called before the app startup.
I added tests but they might not pass, I will take a look at all existing test when its merged on develop (I could not run npm run test until the last Meteor update to 1.7.0.5 due to the Babel issue).

@SachaG
Copy link
Contributor

SachaG commented Sep 28, 2018

What's registerMutation and registerDefault? Also I don't think we necessarily need to wrap registerWatechedMutation with Meteor.isClient. If it's called on the server it just won't do anything since the watched mutation will never be called. But I'd rather avoid using Meteor.isClient/isServer (or Meteor.anything actually)?

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

Successfully merging this pull request may close these issues.

2 participants