Performance improvements, and better lifecycle support

[jbaxleyiii]

This PR does a few things (it can be split up if needed).

• Queries re run on when the redux state (aside from apollo) change
• mapMutationsToProps rebuild on change
• update Changelog.md
• diff queries before rerunning to see if they have changed

Fixes #41 and #20

cc @stubailo @johnthepink

[stubailo]

I think I would prefer { ownProps, state } to be the first argument, not the last, because then it's a lot more consistent to find. I think this might get messy when you start having optional arguments in the mutations.

diff queries before rerunning to see if they have changed

What does this mean?

[stubailo]

@jbaxleyiii wait, did we ever decide that rebuilding the mutation functions on each re-render wouldn't work? Is there a downside to that approach, which wouldn't require changing the API to add extra arguments?

[jbaxleyiii]

@stubailo since we rerun the mapQueriesToProps whenever prop or state changes, I am adding in a check to see if the resulting queries have changed. If not, I don't unsubscribe and rebuild them. So its a performance check to not handle all of the query lifecycle again for no reason.

I can rebuild the mutation functions on each re-render (I have it mostly done). The reason I diverged from that to the { state, ownProps } as arguments was around the use case. Since mutations don't happen on their own (i.e. have to be called within the child component), I thought it would be more efficient to pass the state and props only at execution of the mutation. We can make them first or last for sure. My thought with last was in cases like so:

mapMutationsToProps = () => ({
  action: (my, args) => { // MutationObject }
})

// vs unused `{ ownProps, state }` 
mapMutationsToProps = () => ({
  action: ({ ownProps, state }, my, args) => { // MutationObject }
})

Thoughts?

[deoqc]

Thx for the update. Any interface that you guys decide is just great, but if I was to do it maybe would be with something like:

const mapMutationsToProps = () => ({ state, ownProps }) => ({
  action: (my, args) => { ... }
});

It is much closer to the original one, it feels more natural...

Also, arguments called in runtime my and args (not sure if this is the case, but it is like raw in the docs right?) should be separate from state and ownProps, and like this seems clearer to me.

[jbaxleyiii]

Thanks for the feedback! I think that's one of the points behind this change in my mind. my and args are called at runtime, but you also want the current props and state to mutation execution as well.

Keeping it as the current API is possible! I'll do a comparison branch and try to add in a performance test to see if my concerns about rebuilding the mutations each time is valid or not! (We are rebuilding queries each time so it's probably nothing!)

[jbaxleyiii]

Ehhh, I'll just rewrite to keep the same API. The plumbing is there already and the execution won't be that much, if any perf loss.

[stubailo]

Sounds good to me!

[stubailo]

Why is this now MapQueriesToPropsOptions instead of MapMutationsToPropsOptions?

[jbaxleyiii]

haha because I copy pasted wrong. Thanks!

[stubailo]

@jbaxleyiii looks great to me!

[deoqc]

I think something broke =[. When I upgrade to 0.3.4 it is taking away too much time to start the app or really do anything at all. The xcode console (I'm using react native) is just spiting this:

equalObjects@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69878:31
baseIsEqualDeep@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69668:20
baseIsEqual@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69610:23
equalObjects@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69878:31
baseIsEqualDeep@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69668:20
baseIsEqual@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69610:23
equalObjects@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69878:31
baseIsEqualDeep@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69668:20
baseIsEqual@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69610:23
isEqual@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:70038:19
http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69118:12
dispatch@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:68541:13
dispatch@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:82792:21
http://localhost:8081/index.ios.bundle?platform=ios&dev=true:71511:21
dispatch@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:68892:17
fetchQuery@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:73879:20
startQuery@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:73933:16
http://localhost:8081/index.ios.bundle?platform=ios&dev=true:73777:29
subscribe@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:78901:58
subscribe@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:73702:39
handleQueryData@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69220:40
subscribeToAllQueries@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69163:21
componentWillMount@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:69085:27
mountComponent@http://localhost:8081/index.ios.bundle?platform=ios&dev=true:15996:24

[stubailo]

Could this be deep equal failing on a circular reference or something?

[jbaxleyiii]

@deoqc could you make a reproduction repo I can debug?

[jbaxleyiii]

It may be a circular reference, or the way I'm rebuilding queries when props / state change is getting stuck in a loop

[stubailo]

Also @deoqc can you post a new issue for this?

[jbaxleyiii]

@deoqc that's definitely a bug!