Skip to content
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

networkStatus doesn't change for refetchQueries #1263

Closed
rafgraph opened this issue Feb 4, 2017 · 11 comments
Closed

networkStatus doesn't change for refetchQueries #1263

rafgraph opened this issue Feb 4, 2017 · 11 comments
Labels

Comments

@rafgraph
Copy link

rafgraph commented Feb 4, 2017

Intended outcome:
networkStatus should change to 4 when a query is refetched after a mutation.

Actual outcome:
networkStatus remains at 7 and the query returns cached data (the query then returns the updated data from the refetch also with a networkStatus of 7),

How to reproduce the issue:
Mount CompAWithData which causes queryA to load and prime the cache. Unmount CompAWithData. Run some mutation that does 3 things: 1) changes the data that queryA will return, 2) on success it will cause CompAWithData to mount, and 3) also has refetchQueriers option to refetch queryA.

When CompAWithData mounts as a result of the mutation the initial data it gets from queryA is the cached version but the networkStatus is still 7, it then receives the updated data from the refetch also with a networkStatus of 7.

Note
I think this also happens without the mount and unmount, i.e. a single component with a mutation and a query where the mutation refetches the query. I added notifyOnNetworkStatusChange: true to the query options, which I think should cause the query to pass in the new networkStatus to the component as soon as the refetch is called after the mutation, but nothing is passed into the component until the refetch returns the new data. I'm not sure if that is the intended behavior of notifyOnNetworkStatusChange: true, so I typed it up the mount and unmount way. Is that the intended behavior of notifyOnNetworkStatusChange: true? (docs are a bit light on the topic)

Using apollo-client 0.8.1 and react-apollo 0.9.0

@srosset81
Copy link

I have the same problem with the fetchMore() function, the networkStatus is always set at 7. I have passed the notifyOnNetworkStatusChange: true option.

I'm using apollo-client 0.6.0 and react-apollo 0.8.3

@helfer
Copy link
Contributor

helfer commented Feb 10, 2017

@srosset81 For fetchMore this is expected since we didn't implement the networkStatus update for it yet.

@calebmer
Copy link
Contributor

@RAFREX can we see your mutation’s code for this? There are some cases where we don’t currently expect network status to change.

@rafgraph
Copy link
Author

@calebmer, there are two mutations where it happens:

mutation createPortoflio($input: CreatePortfolioInput!) {
  createPortfolio(input: $input) {
    changedPortfolio {
      id
      ...
      coverImage {
        id
        ...
      }
    }
  }
}

and

mutation updatePortfolio($input: UpdatePortfolioInput!) {
  updatePortfolio(input: $input) {
    changedPortfolio {
      id
      ...
    }
  }
}

@rafgraph
Copy link
Author

rafgraph commented Feb 10, 2017

@calebmer, for notifyOnNetworkStatusChange, if there is a single component with a mutation and a query, where the mutation refetches the query, and the query has notifyOnNetworkStatusChange: true. Is the intended functionality for the component to receive new props with a networkStatus of 4 (and cached data) as soon as the refetch is called?

@calebmer
Copy link
Contributor

@RAFREX It depends on how you are using refetchQueries. If you are referring to a query by name in refetchQueries, then yes. Although we are deprecating that behavior. If you are passing a query document and variables directly to refetchQueries then there is no way to associate that loading state back with your query. This is intentional. Relay does this as well, so it is also precedented.

If you want to show a loading indicator then I recommend setting some loading value to true somewhere in your component’s state.

I really wanted to see when I asked for your mutation code is what you had for refetchQueries 😊

@rafgraph
Copy link
Author

@calebmer I'm passing in the query document and variables to refetchQueries.

retchQueries: [{
  query: portfolioQuery,
  variables: portfolioQueryVariables(input.urlTitle),
}]

@calebmer
Copy link
Contributor

Yep, then we wouldn’t expect loading to be set to true 😣

Is this something that you really need from Apollo Client? It might be easy enough to implement yourself for the UI that needs it.

@helfer
Copy link
Contributor

helfer commented Feb 10, 2017

@RAFREX if your other component is active at that moment (which it seems to be since you expect it to update), you could just refer to the query by the name you gave it for now. That's the quick solution, but I think for longer term it's better to follow @calebmer's advice and implement it yourself in the UI.

@helfer helfer closed this as completed Feb 10, 2017
@rafgraph
Copy link
Author

rafgraph commented Feb 10, 2017

Thanks @calebmer and @helfer
In the single component example, I agree there is no need for Apollo to track the loading state (I don't have that situation in my app, I was just trying to come up with minimal reproduction case).

Where it is needed, is when the mutation causes a different route to match, which loads a different set of components that has the query that is being refetched from the mutation. So it's not possible to track the loading state locally or pass it down through props, I could track loading globally, but it would be great if Apollo would take care of that. Basically, this is what I'm dealing with:

How to reproduce the issue:
Mount CompAWithData which causes queryA to load and prime the cache. Unmount CompAWithData. Run some mutation that does 3 things: 1) changes the data that queryA will return, 2) on success it will cause CompAWithData to mount, and 3) also has refetchQueriers option to refetch queryA.

@joshjg
Copy link
Contributor

joshjg commented Aug 11, 2017

@helfer @calebmer Why can't Apollo support this? It seems like it would be very tedious, or maybe even impossible, to implement this ourselves. We can set loading: true somewhere in our state, but how do we know when the refetch is done, so we can set loading: false?

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

No branches or pull requests

5 participants