@tmeasday,
I went back and played around with time travel a bit. What seems to be popping out of the blue when I reset my store are APOLLO_QUERY_STOP
actions. I'm not sure if these should be happening or not. Maybe you can shed some light on this?
Nevertheless, I'm realizing that time traveling a react-apollo
app isn't really possible because data from the graphql
higher order component gets called on componentDidMount
. This means that as you try to reproduce each step, once you get to the step that would mount the GraphQL connected component, the queries are going to fire off, thus overlapping with the next actions one would manually dispatch while time traveling.
It's definitely possible to reduce the state from all the actions though. 馃槃
EDIT: Starts where this comment left off.
I went back and played around with time travel a bit. What seems to be popping out of the blue when I reset my store are APOLLO_QUERY_STOP actions. I'm not sure if these should be happening or not. Maybe you can shed some light on this?
I can't really see why that would happen. That action seems to only get fired by unsubscribing to the query -- are you maybe doing that?
Nevertheless, I'm realizing that time traveling a react-apollo app isn't really possible because data from the graphql higher order component gets called on componentDidMount. This means that as you try to reproduce each step, once you get to the step that would mount the GraphQL connected component, the queries are going to fire off, thus overlapping with the next actions one would manually dispatch while time traveling.
Are we talking about nested graphql components here? I could see that happening maybe, but I'm not totally understanding how that flow could come about otherwise (why would the component re-mount on query results otherwise?)
If it is the nested case, it's a bit of a doozy. I wonder if you have any ideas about how we could resolve this?
I can't really see why that would happen. That action seems to only get fired by unsubscribing to the query -- are you maybe doing that?
Ok, so if APOLLO_QUERY_STOP
is fired when the query is unsubscribed then it would make sense for it to happen when the store is reset.
store is reset
-> selected props change
-> component with query unmounts
-> query is unsubscribed
-> `APOLLO_QUERY_STOP
action is fired`
I didn't expect the query to be subsribed even after it was fulfilled. I thought after APOLLO_QUERY_RESULT
or APOLLO_QUERY_RESULT_CLIENT
came back the query would be unsubscribed.
Are we talking about nested graphql components here? I could see that happening maybe, but I'm not totally understanding how that flow could come about otherwise (why would the component re-mount on query results otherwise?)
It's not nested components. It's a react native app and there's navigation involved. When navigating from the login screen to the home page (connected with graphql
higher order component) the query is automatically fired because apollo-react
subscribes the query when the component mounts (I'm assuming this).
When I use Redux DevTools to play around with my store I start seeing the weird behavior. (e.g., APOLLO_QUERY_STOP
gets called when a component is unmounted and queries get fired when a component gets mounted)
By the way, would you say nesting GraphQL components is an anti-pattern and that there should only be one top level query that gets everything and passes down the props as needed?
I originally thought I could take advantage of the cache and nest the components and the nested ones wouldn't have to do duplicate queries, but it turns out that doesn't work as well as one would expect.
I didn't expect the query to be subsribed even after it was fulfilled. I thought after
APOLLO_QUERY_RESULT
orAPOLLO_QUERY_RESULT_CLIENT
came back the query would be unsubscribed.
RA needs to keep watching the query so later results to the query (from mutations or polling or whatever) can be reflected in your UI.
It's not nested components. It's a react native app and there's navigation involved. When navigating from the login screen to the home page (connected with
graphql
higher order component) the query is automatically fired becauseapollo-react
subscribes the query when the component mounts (I'm assuming this).
Oh, so you are saying other redux actions you time-travel through are causing the component to get mounted? I guess that's more or less the same problem as the nested queries one if you think about it.
I wonder if dispatching actions from component lifecycle is an anti-pattern in Redux. It seems like you'd inevitably run into this problem when time-travelling. Although I've no real idea how you'd do it otherwise.
/me is going on a google journey now.
By the way, would you say nesting GraphQL components is an anti-pattern and that there should only be one top level query that gets everything and passes down the props as needed?
No, I don't think so!
I originally thought I could take advantage of the cache and nest the components and the nested ones wouldn't have to do duplicate queries, but it turns out that doesn't work as well as one would expect.
Tell me more about this. I don't think these patterns have been completely settled yet.
Oh, so you are saying other redux actions you time-travel through are causing the component to get mounted? I guess that's more or less the same problem as the nested queries one if you think about it.
Yes, a case would be a simple nav push, like dispatch(NavigationActionCreators.push('home')
. That will push an action that will cause a new scene to get rendered (i.e., component mounted) which will then trigger a GraphQL query to populate the feed in "home".
I wonder if dispatching actions from component lifecycle is an anti-pattern in Redux. It seems like you'd inevitably run into this problem when time-travelling. Although I've no real idea how you'd do it otherwise.
The only thing that comes to mind in this case would be to go extreme redux and simply dispatch a thunk (or start a saga) when the component mounts and then let it handle asynchrony from there and fire the actions that change the store as it goes. This still would happen automatically when the component mounts, though, so it really doesn't sole our problem. The only way to prevent this would be to have some sort of "time-travel mode" where thunks aren't added to the time travel list of actions... and actually, now that I think about it, I think that's the case.
So let me expand on this: If instead of EDIT: Nevermind, the thunk would still get dispatched on mount, so all the actions dispatched by it would get dispatched. I'm not sure there is actually a way unless there's custom setting ("time-travel mode"?) that will entirely ignore certain actions (thunks maybe). REEDIT: Basically, if the logic is moved to Redux land, then implementing the "time-travel mode" would be very easy. If the logic is tied to the component then it's not possible.react-apollo
subscribing on componentDidMount
it simply dispatched a thunk and the subscription logic was decoupled from the graphql
higher order component (it happened in Redux land, triggered by the thunk), then it would be very easy to ignore this action when time traveling (time traveling ignores thunks by default), thus solving our issue with breaking time-travel.
Tell me more about this. I don't think these patterns have been completely settled yet.
Of course! 馃槃
So I'm building a production React Native app that uses GraphQL and Apollo. I'm running into a lot of real-world use cases so I wouldn't mind sharing what I've come through/learned. For example, I know you're still missing a Relay example in the docs for pagination in react-apollo
. I'm doing queries like that myself so perhaps I could contribute to the example once I finish the busy task of getting this shipped.
Anyway, to answer your question. Sometimes nested components can get the necessary props from the parent but other times the queries are completely different and the caching system won't help you. For example, if you have a query called comments
that returns [Comment!]!
and another one called comment
whose signature is comment(id: ID!): Comment
, there is no way of reconciling the responses of queries to comments
with those of comment
. Even if you use dataIdFromObject
, that's only good for updating the cache, there is no way to tell apollo, "hey, all the ids that were returned from comments
actually map to ROOT_QUERY.comment(id: $id), where $id is the one we just got from dataIdFromObject
.
Another thing that I haven't looked much into is that I think that the nested components mount first (before their parent components), so it becomes awkward to fetch a lot of fields from the nested components simply to cache them. Anyhow, as I keep building the app I'll keep finding better ways of approaching each issue.
I hope this is helpful.
So let me expand on this: If instead of react-apollo subscribing on componentDidMount it simply dispatched a thunk and the subscription logic was decoupled from the graphql higher order component, then it would be very easy to ignore this action when time traveling (time traveling ignores thunks by default), thus solving our issue with breaking time-travel.
Is that true? (That TT ignores thunks). What is the mechanism for that? I wonder if there's an equivalent approach we can take.
So I'm building a production React Native app that uses GraphQL and Apollo. I'm running into a lot of real-world use cases so I wouldn't mind sharing what I've come through/learned. For example, I know you're still missing a Relay example in the docs for pagination in react-apollo. I'm doing queries like that myself so perhaps I could contribute to the example once I finish the busy task of getting this shipped.
That'd be awesome!
For example, if you have a query called comments that returns [Comment!]! and another one called comment whose signature is comment(id: ID!): Comment, there is no way of reconciling the responses of queries to comments with those of comment
That makes sense; there's not really any way around that. I do wonder what the use case is for a separate query for the comment however (I am imagining a list of comments, but maybe I'm off base here?).
Another thing that I haven't looked much into is that I think that the nested components mount first (before their parent components)
Wouldn't the nested component depend on the result of the parent component? I would have thought if that is the case they would have to wait on the parent's query. But maybe we need to be more concrete in the example..
Is that true? (That TT ignores thunks). What is the mechanism for that? I wonder if there's an equivalent approach we can take.
I'm sorry for using the wrong nomenclature here. The Redux DevTools store enhancer will only track dispatched plain actions (POJOs), meaning that thunks (functions) will not get recorded. I'm not 100% on this but I'm pretty sure I read about it a while ago. I tried to find the source but no luck. The idea is similar to putting redux-logger
as the last middleware so that it doesn't pick up thunks. In this case, instead of forcing the developer to put it last, the middleware doesn't record if typeof action === 'function'
. Honestly, I've got to do more research on this.
That makes sense; there's not really any way around that. I do wonder what the use case is for a separate query for the comment however (I am imagining a list of comments, but maybe I'm off base here?).
Imagine having a feed of items, lets say tweets. The query to get the feed is analogous to comments
and the query to get the details of a tweet is analogous to comment
. In GraphQL you can get all the details from comments
simply by specifying the fields but it would be nice to separate concerns and have the Feed
component only query all the IDs of each tweet and then have each individual FeedItem
component query the details of the tweet. To be honest though, you probably wouldn't do this. I was doing this initially in our app's feed and it was rendering weirdly (some requests came earlier than others causing rendered components to jump around). Also, you kind of only want to do 1 trip to the server, even if it means a much bigger response that takes longer. Ultimately it depends what you want to optimize for. I'm looking forward to the release of @defered
to be able to defer some parts of the query and render the most important parts first.
Wouldn't the nested component depend on the result of the parent component? I would have thought if that is the case they would have to wait on the parent's query. But maybe we need to be more concrete in the example..
You're right, the child component probably the parent to be there to be able to mount in the first place.
Forgot to mention, I edited my comment above. Not sure if you saw the edits.
@tmeasday,
By the way, if you want me to, I'd be happy to go into more detail into what I mean by having the query logic live in the Redux side.
Re: nested queries:
I think the better approach here is to use a fragment in this case (where you want to separate the concerns about the structure of the comment query). Then the feed can grab the fragment off the comment and include it in the query. Our fragment support is pretty raw and we don't really have great patterns for this but it's something we actively want to explore and figure out a solution to.
Probably something less intense than what Relay does is where we'll end up.
By the way, I'd be happy to go into more detail into what I mean by having the query logic live in the Redux side.
The part I can't figure is how if your component dispatches in its lifecycle callbacks (via thunks or otherwise) how you could avoid extra actions (forget network requests for a moment) occurring when the component is rendered during TT, unless there's some special mechanism where Redux stops dispatching or something.
I too need to do a lot more research in this area but it's something I do want to look at in a lot more detail soon. I'll report back as I figure stuff out.
I think the better approach here is to use a fragment in this case (where you want to separate the concerns about the structure of the comment query). Then the feed can grab the fragment off the comment and include it in the query. Our fragment support is pretty raw and we don't really have great patterns for this but it's something we actively want to explore and figure out a solution to.
You're right! Fragments are exactly what I need.
Probably something less intense than what Relay does is where we'll end up.
Have you seen what lokka does for fragments?
The part I can't figure is how if your component dispatches in its lifecycle callbacks (via thunks or otherwise) how you could avoid extra actions (forget network requests for a moment) occurring when the component is rendered during TT, unless there's some special mechanism where Redux stops dispatching or something.
You're exactly right about Redux having to stop the dispatching. Basically, "time-travel mode" could be activated by dispatching an action ACTIVATE_TIME_TRAVEL
. Once this is dispatched, the "time-travel" middleware could kick in, at which point only POJOs can be dispatched. If a thunk is dispatched it is ignored (maybe still logged but ignored). This way one can time travel without worrying about side effects resulting from asynchrony or thunks. When you're done you can simply dispatch DEACTIVATE_TIME_TRAVEL
. Makes sense?
I know this is completely outside the scope of Apollo now, but I'm enjoying the discussion! 馃槃
You're exactly right about Redux having to stop the dispatching. Basically, "time-travel mode" could be activated by dispatching an action ACTIVATE_TIME_TRAVEL. Once this is dispatched, the "time-travel" middleware could kick in, at which point only POJOs can be dispatched. If a thunk is dispatched it is ignored (maybe still logged but ignored). This way one can time travel without worrying about side effects resulting from asynchrony or thunks. When you're done you can simply dispatch DEACTIVATE_TIME_TRAVEL. Makes sense?
If I was implementing this from scratch I would do the first half of what you said but when replaying actions attach an extra __fromTT__: true
field; then I would add a middleware that just dropped actions on the floor that didn't have that field. Maybe that's what redux dev tools does? I should look...
Redux DevTools actually doesn't do that. Maybe it's something they should implement? It sounds like that should be the functionality out of the box given that when you're time traveling you wouldn't want to dispatch extraneous actions.
Pinging @zalmoxisus and @gaearon.
If I was implementing this from scratch I would do the first half of what you said but when replaying actions attach an extra fromTT: true field.
The extension provides a getMonitor > isTimeTraveling
function you can use for that purpose. You can add a __fromTT__
parameter via a middleware like so or you can just use a global variable for debugging. This API is still experimental so any suggestions are welcome.
UPDATE: I just read that it's about React Native. So you're probably using Remote Redux DevTools?
Basically, "time-travel mode" could be activated by dispatching an action ACTIVATE_TIME_TRAVEL. Once this is dispatched, the "time-travel" middleware could kick in, at which point only POJOs can be dispatched. If a thunk is dispatched it is ignored (maybe still logged but ignored). This way one can time travel without worrying about side effects resulting from asynchrony or thunks. When you're done you can simply dispatch DEACTIVATE_TIME_TRAVEL. Makes sense?
Yes, we want to implement something like this to lock all the changes while time travelling. Would that help?
@zalmoxisus, thanks for chiming in!
I think that exposing isTimeTravelling
is great. It's very easy to implement middleware that ignores any actions that aren't from time travel.
I guess my question is, do we want the dev to handle this or should there be a switch in the DevTools monitor that when on will simply ignore any actions not triggered by the monitor?
What I mentioned above, regarding ignoring anything not a POJO, doesn't really work well because a POJO sent in componentWillMount can still trigger side effects (i.e., via middleware). The correct approach would be to simply ignore any action that doesn't come from the monitor, given a certain predicate (i.e., isMonitorAction
)
EDIT: And it turns out you already expose such predicate! 馃槃
@zalmoxisus,
Also, I really love the work you've been doing to handle errors remotely and in realtime as they happen in production. It would be impossible to do this though without suppressing side effect actions from componentWillMount
, other middleware, etc.
EDIT: For the remote user that's getting their app debugged, once time travel mode kicks in, they basically wouldn't be able to do anything in the app (as long as all app interactions are handled by Redux).
@migueloller, thanks for the feedback!
I guess my question is, do we want the dev to handle this or should there be a switch in the DevTools monitor that when on will simply ignore any actions not triggered by the monitor?
Redux DevTools enhancer knows nothing about action creators. It recomputes only reducers. That said, if the dev calls an action creator, for example from componentDidMount
, we can prevent only invoking its reducers. Anyway we could prevent side effects from middlewares like redux-saga
. That's exactly what we want achieve in https://github.com/zalmoxisus/redux-devtools-instrument/pull/8. The initial idea was just to froze dispatching till unlocking it, but I guess it will not suit you use case, so better we'll cancel dispatching new actions at all while it's locked.
@zalmoxisus,
Yes, being able to just drop all actions that don't come from the monitor would be awesome! It would allow for time travel without having to worry about actions springing up from componentWillMount
or other middleware that dispatch actions (redux-saga
is one of these, but basically any middleware could do this).
Thanks for the prompt responses!
@migueloller, I've just published [email protected]
and [email protected]
, which support this feature. We still need to update the UI (add a button) on remotedev-app
(and the extension), however you can get it work just by calling in your app:
store.liftedStore.dispatch({ type: 'LOCK_CHANGES', status: true });
To unlock changes:
store.liftedStore.dispatch({ type: 'LOCK_CHANGES', status: false });
Please let me know whether it helps.
Seems pretty great!
My naive thought (and I'll admit I haven't spent a lot of time thinking about this), was that when the user time travelled, the TT mechanism would automatically lock dispatches until rendering is done (maybe a small timeout to be safe).
Then if the component sticks to dispatching single actions (which could be picked up by something like redux saga), or thunks, then it will be "locked out" during React's component lifecycle and everything should work.
If the component doesn't follow that pattern, and instead initiated network calls / something else that dispatched actions after timeouts (this is what Apollo is doing right now I think), then it wouldn't work; but I suspect the above way is more idiomatic Redux anyway, right?
@zalmoxisus, awesome! Definitely helps. Now I'm just waiting for the button in the monitor so that I don't have to use the dispatcher. 馃槈
@tmeasday, yeah, it looks like blocking all actions while time traveling is the best way.
Thanks all for the great discussion. I'm satisfied with the outcome and am going to close the issue. 馃槃
@migueloller is there still an issue that Apollo dispatches actions in the "wrong" way and this mechanism won't (for instance) stop network requests when you are TT-ing?
@tmeasday, nope. Apollo never did anything wrong really. I just got confused with actions getting fired on componentDidMount
and thought it was an Apollo issue, when really it's unavoidable for any action that is dispatched as the app goes along (either a component mounts, a user clicks a button, something times out, etc.). Will the changes @zalmoxisus made time travel will simply ignore all actions so it's all good.
The one thing that could be "wrong"/annoying is all the APOLLO_QUERY_STOP
actions, But I believe those are dispatched only when a subscribed component dismounts and the subscriptions should never happen anyway when time traveling.
Does dispatching APOLLO_QUERY_STOP
actually do something or changes some behavior or is it purely for logging purposes?
@migueloller but am I not correct in saying when a graphql
container renders, the following will occur?:
APOLLO_QUERY_INIT
is dispatched (this will get squashed if the store is "locked")APOLLO_QUERY_RESULT
firesAside from the unnecessary network requests, it seems like that APOLLO_QUERY_RESULT
could be an issue.
I would have thought the APOLLO_QUERY_STOP
would be OK -- it fires immediately when the component is unmounted, which would be during the "lock" period.
Does dispatching APOLLO_QUERY_STOP actually do something or changes some behavior or is it purely for logging purposes?
It removes the query status from the store. In the future you could imagine it might remove the query results or mark them to be GC-ed.
@tmeasday and @zalmoxisus,
Ok, so there are 2 different things we're talking about here.
The first one is what @zalmoxisus just mentioned which is to be able to lock all changes to be able to work on some feature. From seeing this line though, it looks like it's not made for time travel, but more to simply lock the entire store so that nothing happens. So it looks like this specific change won't solve our problem.
The second is one is what @zalmoxisus had mentioned in a previous comment regarding the isTimeTraveling
field from the object returned by getMonitor()
here. That does look like it would help with what we're trying to accomplish. To do this though, we would need to implement a custom tool that would check if the isTimeTraveling
is true, and if it is then to ignore all actions that aren't from the monitor.
@zalmoxisus, do you see how these are 2 different use cases? I could even potentially see 2 different buttons in the monitor for them. One to drop all actions and another to drop all actions that don't come from the monitor.
@tmeasday, to answer your question:
If that is the way that Apollo currently handles it, then yes, it will interfere with time travel. But this will only happen if the store is unlocked (i.e., accepting non-time-travel actions) when the result comes back. This is unlikely. It is possible to make it 100% foolproof though, doing it with super idiomatic Redux. To make it so, it would be necessary to have a custom Apollo middleware that initiates the network process. See redux-api-middleware for an example. With middleware like that, the action that calls the API would never be dispatched, so the network request wouldn't be made and the result wouldn't be a problem.
I think that if Apollo implemented something like this internally it would definitely be cleaner. Then basically, what a connected GraphQL component does is dispatch an action when it mounts, it doesn't even have to worry about unsubscribing when it unmounts because all of that happens internally in Redux land.
@migueloller, could you please elaborate why LOCK_CHANGES
doesn't solve the problem? We want to prevent side effects not only for time travelling, but also for skipping (canceling) actions, for importing states, and most important for hot reloading (which also invokes recomputing all actions).
I could even potentially see 2 different buttons in the monitor for them. One to drop all actions and another to drop all actions that don't come from the monitor.
In case of time travelling, LOCK_CHANGES
does exactly this (drop all actions that don't come from the monitor) because it doesn't recompute anything. In other cases it also allows recomputing actions that was already dispatched.
DevTools enhancer is the last in the compose, and all the middlewares are invoked before it. We need the second enhancer at the beginning. I've just published [email protected]
, where you can use composeWithDevTools
like in the example. It should drop actions before invoking any middlewares when it's locked. Please give it a try.
However, I guess you want to prevent invoking network requests not from middlewares (as it would be in case of redux-saga
), but from the subscribed functions, which I'm not sure how to handle better.
Update: What if we'll expose a global variable like window.__REDUX_DEVTOOLS_IS_LOCKED__
, which you could check (process.env.NODE_ENV === 'production' || typeof window !== 'object' || !window.__REDUX_DEVTOOLS_IS_LOCKED__
) before invoking network requests? The reason why we cannot make isTimeTraveling
global is that it's just for a specific instance (store), but here we're locking all the stores.
@zalmoxisus,
I apologize, I thought that LOCK_CHANGES
would drop EVERY action, including the ones from the monitor. If actions from the monitor aren't dropped then it works perfectly.
@zalmoxisus I'm finally caught up on this. I tried using the store.liftedStore.dispatch({ type: 'LOCK_CHANGES', status: true });
etc above but it didn't seem to quite work as I thought it would.
I created a really simple example that tries to simulate how apollo-client
might behave to help with this [1]. The basic flow is
componentDidMount
).componentWillUnmount
).I definitely see issues when I time travel around in that because replaying actions makes the left pane appear and disappear, and new subscription and teardown actions occur.
Locking does not appear to stop that happening -- it does however seem to lock the UI.
Is this how you imagine a dev-tools compatible Redux based network layer might operate? Is there a better way to think about it?
[1] Code is here, there is a bit of other react-persist
stuff in there but I think it's not affecting this: https://github.com/tmeasday/test-redux-app
@tmeasday, thanks for the example!
I've just tried it, and see the issue with time travelling, but everything works fine when it's locked (after store.liftedStore.dispatch({ type: 'LOCK_CHANGES', status: true });
): no actions dispatched and no running subscribe thunk
in the console.
Am I missing something?
@zalmoxisus you know what, you're right! I don't know what I did before, but when I tried again it worked exactly as I hoped. Great! I think we are on the same page then.
So would your opinion be that driving network operations via thunks is the way to go? It seems like it to me.
Thunks would be ok, as well as other middlewares. You could also have your custom middleware like in the real-world example. Locking will block all middlewares and store enhancers.
In case of having network request from store.subscribe
or from the connected React components, one should prevent side-effects by himself. We'll expose window.__REDUX_DEVTOOLS_EXTENSION_LOCKED__
for that.
In future we could autolock the store while timetravelling.
Thanks for investigating this!
Locking was implemented in Redux DevTools Extension and in Remote Redux DevTools. Here's a blog post with more details.
@tmeasday, thinking over, I don't think we should force users to add redux-thunk
middleware, better to execute side effects right from (new ApolloClient()).middleware()
for specific action types:
function clientMiddleware(store) {
return next => action => {
const result = next(action);
// ...
if (action.type === 'SUBSCRIBE_REQUEST') {
// network requests or other sideffects
// here we can dispatch also: next({ type: 'SUBSCRIBE_SUCCESS' })
// or: next({ type: 'SUBSCRIBE_FAILED' })
} else if (action.type === 'TEARDOWN_REQUEST') {
// network requests or other sideffects
}
return result;
};
}
And from the component:
componentDidMount() {
this.props.dispatch({ type: 'SUBSCRIBE_REQUEST' });
}
componentWillUnmount() {
this.props.dispatch({ type: 'TEARDOWN_REQUEST' });
}
@zalmoxisus,
Yes! That is the way to go.
This issue has been automatically closed because it has not had recent activity after being marked as stale. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to Apollo Client!
Most helpful comment
@migueloller, I've just published
[email protected]
and[email protected]
, which support this feature. We still need to update the UI (add a button) onremotedev-app
(and the extension), however you can get it work just by calling in your app:To unlock changes:
Please let me know whether it helps.