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

Add networkStatus prop to connected components #322

Closed
cannoneyed opened this Issue Nov 11, 2016 · 11 comments

Comments

Projects
None yet
6 participants
@cannoneyed
Copy link

cannoneyed commented Nov 11, 2016

Steps to Reproduce

When fetching more data, using a pattern identical to that proposed in the docs, the loading prop that is passed down to the wrapped component is false after fetchMore is invoked.

const Query = gql`
query paginatedQuery ($offset: Int) {
    paginatedItems (offset: $offset) {
        ... item
    }
}
`
@graphql(Query, {
    props:  ({ data }) => {
        const { fetchMore } = data
        const queryResult = data.paginatedItems

        data.loadNextPage = () => fetchMore({
            variables: {
                offset: queryResult.length
            },
            updateQuery: (prev, { fetchMoreResult }) => {
                if (!fetchMoreResult.data) {
                    return prev
                }

                return Object.assign({}, prev, {
                    [queryName]: [
                        ...prev[queryName],
                        ...fetchMoreResult.data.paginatedItems
                    ]
                })
            }
        })

        return { data }
    }
}
class View extends Component {
    render() {
        const { data: { paginatedItems, loading, loadNextPage } } = this.props
        // Loading is false after loadNextPage is invoked
    }
}

Buggy Behavior

It seems the loading prop is only ever tied to the initial load of data. For simple components that don't need to update or paginate, this is fine. However, the workaround for this (managing refetch / loadMore status with local component state) is not ideal.

Expected Behavior

The following PR apollographql/apollo-client#707 in apollo client exposes a networkStatus object for handling the status of non-initial-load network requests, such as refetch, fetchMore, and subscriptions.

It would be very nice to have the networkStatus object exposed as a prop to wrapped components (perhaps if specified as an option), so that paginated / more complex query operations loading/error state can be managed by the react-apollo wrapper rather than using local ad-hoc local component state.

Version

  • apollo-client@0.5.0
  • react-apollo@0.5.16
@helfer

This comment has been minimized.

Copy link
Member

helfer commented Nov 12, 2016

@cannoneyed FYI, the fetchingMore network status isn't implemented in apollo-client yet. All others should be.

@wzrdzl

This comment has been minimized.

Copy link
Contributor

wzrdzl commented Nov 14, 2016

I would like to implement this functionality since my app depends on it. @jbaxleyiii @cannoneyed @helfer Is anybody working on it now?

@cannoneyed

This comment has been minimized.

Copy link
Author

cannoneyed commented Nov 15, 2016

@wizardzloy I'm starting to look into this now.

@cannoneyed

This comment has been minimized.

Copy link
Author

cannoneyed commented Nov 15, 2016

@helfer @wizardzloy So what I've found so far is that there seems to be some work needed in apollo-client... it looks like when ObservableQuery.fetchMore is called, a new query is created but no corresponding listener is tied to it. All of the networkStatus stuff is being handled correctly, it's just that it's all happening on a separate query, and the only time where the connected component updates is when the original query has its data merged with the new one (in updateQuery).

This is an interesting situation. On the one hand, it totally makes sense that a fetchMore would create a new query, since it is in fact a separate, different query, with different variables and different statuses (the original one has loaded, and the new one is loading). However, it's kind of a bummer that the connected component doesn't currently have any way of knowing what the status of the "sibling" fetchMore query is. Perhaps the best way to handle this would be to expose some sort of prop that has some notion of these "sibling" queries. I'm not exactly sure what that might look like yet, but it seems like this would be very useful for more effectively managing components with multiple queries (ie paginated components)

@nicolo-paganin

This comment has been minimized.

Copy link

nicolo-paganin commented Nov 26, 2016

+1 this feature will be very useful

@helfer

This comment has been minimized.

Copy link
Member

helfer commented Dec 13, 2016

@cannoneyed I think we can implement this kind of tracking internally in Apollo Client and expose it just through a fetchingMore network status. Would that be enough information for your use-case?

@czert

This comment has been minimized.

Copy link

czert commented Dec 14, 2016

@helfer It would be for mine :)

@czert

This comment has been minimized.

Copy link

czert commented Mar 31, 2017

We've had the data.networkStatus prop for a while, and until 1.0.0 it worked beautifully. While "fetching more", its value was 3. In 1.0.0, though, its value doesn't change back from 7.

Is this a known effect of some other change that I missed, or should I open an issue for this?

@helfer

This comment has been minimized.

Copy link
Member

helfer commented Apr 7, 2017

@czert are you using the query option notifyOnNetworkStatusChange? If not, the component wouldn't be notified of changes in network status unless the data has also changed. If you are using that option but still aren't seeing the network status update, then it's a bug and a repro would be much appreciated! 🙂

@czert

This comment has been minimized.

Copy link

czert commented Apr 7, 2017

@helfer thanks, I wasn't using the new notifyOnNetworkStatusChange until a different issue brought it to my attention. All works fine now, thanks! :)

@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented May 3, 2017

networkStatus is included in props now

@jbaxleyiii jbaxleyiii closed this May 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment