Skip to content
This repository has been archived by the owner on Nov 20, 2020. It is now read-only.

Feat/optimistic response #69

Merged
merged 2 commits into from
Dec 17, 2019
Merged

Feat/optimistic response #69

merged 2 commits into from
Dec 17, 2019

Conversation

mbirkegaard
Copy link
Collaborator

This adds optimistic response to ApolloHooksMutation and updates the Persons example to show how to correctly construct an optimistic response that apollo can understand.

Comment on lines +15 to +25
type t = {
.
"__typename": string,
"updatePerson": {
.
"__typename": string,
"age": int,
"id": string,
"name": string,
},
};
Copy link
Collaborator Author

@mbirkegaard mbirkegaard Dec 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it it's is now, this should be compatible with all versions of bucklescript, but can be changed to only work with bucklescript >= 7 if that's preferred.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright!

@@ -11,6 +11,34 @@ module EditPersonMutation = [%graphql
|}
];

module OptimisticResponse = {
type t = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment that this is necessary because the types graphql_ppx_re generates are not 1:1 to the JS representation and probably this won't be necessary as soon as teamwalnut/graphql-ppx#20 gets merged?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't have time to check back in before now. Would you still like me to add it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be appreciated! I haven't published yet

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On it.

@fakenickels
Copy link
Member

looks good to me! after that comment we're good to merge

@fakenickels fakenickels merged commit 20103bb into reasonml-community:master Dec 17, 2019
@fakenickels
Copy link
Member

Released under v4.1.0

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

Successfully merging this pull request may close these issues.

2 participants