Graphql-flutter: Move "update" function from flutter component to client

Created on 19 May 2019  路  15Comments  路  Source: zino-app/graphql-flutter

In line with Apollo, we should add an update function as well as a refetchQueries array within the MutationOptions of QueryManager's mutate function,

Most helpful comment

Yeah we should move the callback logic involved into graphql, and just pass through the current update callback. This applies to pretty much all logic - graphql_flutter shouldn't really have much logic in it

All 15 comments

update logic exists but needs to be moved from graphql_flutter
We should also add refetchQueries - but only the part they don't want to deprecate

Right now all "safe" queries are re-resolved from the cache on mutation, so there's that 馃槄

@mainawycliffe Do you have any guidance on this?

@smkhalsa At first I didn't understand what you meant, but now I see the difference between our own implementation and React Apollo implementation. Our Update method is outside mutation options, which makes it only accessible to users using graphql_flutter and not regular graphql. I think it is possible to move the update the method, to allow users of graphql to use the option too. @micimize what do you think?

Thanks @mainawycliffe. My use case requires that I initiate a mutation from my store in response to state changes as opposed to something directly initiated by a user in the UI. As such, I'm using the graphql library directly, not graphql_flutter.

Yeah we should move the callback logic involved into graphql, and just pass through the current update callback. This applies to pretty much all logic - graphql_flutter shouldn't really have much logic in it

@mainawycliffe it sounds like this is a change we want to make. Any further thoughts?

@smkhalsa yea, this is a change we should make, the question is the best approach to achieve this. It will definitely be a breaking change, but if we move the update call back alone, I think we could introduce some API inconsistencies, the best approach, I believe, is to move all the callbacks to MutationsOptions - onError, onComplete, update which is more consistent with Apollo React.

I was working on adding a onError callback, #435, I could add this to that today or tomorrow.

@micimize what do you think about moving the callbacks to MutationOptions instead.

@mainawycliffe yeah that's what I think as well - all functionality should be in graphql and merely exposed in an idiomatic flutter way in graphql_flutter

I will work on this and bundle it with the #435

@mainawycliffe any update on this?

@smkhalsa I have moved the update and other callbacks up to the MutationOptions am just trying to figure the best way to call the callbacks when calling the mutate method directly from the client. That's the only section and updating documentation and adding some tests if any are required.

@smkhalsa can you test the PR (#435) and see if it works as expected

@mainawycliffe I added a few initial comments to the PR.

@smkhalsa Sorry, I had a week packed with conferences I will look into those changes when I am back on Monday.

This issue was resolved by #435

Was this page helpful?
0 / 5 - 0 ratings

Related issues

DrBanner97 picture DrBanner97  路  13Comments

micimize picture micimize  路  16Comments

chimon2000 picture chimon2000  路  24Comments

mainawycliffe picture mainawycliffe  路  16Comments

juicycleff picture juicycleff  路  50Comments