-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Mutation results #320
Mutation results #320
Conversation
9c9dd48
to
37fa7eb
Compare
} | ||
|
||
export type MutationArrayDeleteAction = { | ||
type: 'ARRAY_DELETE'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should prefix with APOLLO_
to be consistent no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't real actions - they end up in the applyResults
section of the MUTATION_RESULT
action, here: https://github.com/apollostack/apollo-client/pull/320/files#diff-162241b86351f640a8bb3127ed5fbd5bR92
This is because we want to process all of the mutation result stuff synchronously in one pass, rather than as multiple actions one after the other - it seems weird to me to fire many actions in one tick, when they represent one event (a mutation arriving from the server).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to not call these actions at all? I was confused at first by this as well. It seems that these are more like "action applications" although I don't think that is a good name for them. I just think calling them "actions" makes it seem as though they are actual actions which will lead to a transition within the Redux store which can be a bit confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm going to call them "behaviors" after thinking about it.
I made a couple of comments but looks good to me overall. Excited for this to get merged! |
db0a113
to
36d4d1c
Compare
I promise I'll bring it back down after this
A good day's worth of work, finally being merged! Feels good. And with good test coverage to boot. Now to write the docs... |
[WIP] Mutation results
See #317 for design, please give feedback.
TODO:
Additional tests needed: