QueryResult.loading not updated when fetchMore is called from 3.0.0-beta.46 #6459
Milestone
Comments
|
See also #6354 |
|
Yep, that's the same problem. Let's close this in favor of #6354. Thanks! |
benjamn
added a commit
that referenced
this issue
Jul 13, 2020
Should fix #6354, #6542, #6534, and #6459. Reliable delivery of loading results is one of the core benefits that Apollo Client strives to provide, expecially since handling loading states tends to be such an annoying, error-prone task in hand-written state management code, and Apollo Client is all about keeping hand-written state management code to a minimum. Ever since I refactored the FetchPolicy system in #6221, the fetchMore method of ObservableQuery has not been delivering a loading state before sending its request. Instead, the observed query was updated only once, after the completion of the fetchMore network request. I've been aware of this problem for a while now, but I procrastinated solving it because I knew it would be, well, annoying. With the final release of AC3 right around the corner (Tuesday!), the time has come to get this right. This loading result doesn't fit neatly into the fetchQueryObservable system introduced by #6221, since the consumer of the loading result is the original ObservableQuery, but the network request gets made with a fresh query ID, using different variables (and possibly even a different query) passed to fetchMore. This separation is important so that we don't have to change the query/variables/options of the original ObservableQuery for the duration of the fetchMore request. Instead, the fetchMore request is a one-off, independent request that effectively bypasses the usual FetchPolicy system (technically, it always uses the no-cache FetchPolicy). In Apollo Client 2.x (and before #6221 was released in beta.46), this logic was guided by an extra fetchMoreForQueryId parameter passed to QueryManager#fetchQuery, which ended up appearing in a number (16) of different places, across three different methods of the QueryManager. I think it's an improvement that the logic is now confined to one block of code in ObservableQuery#fetchMore, which seems naturally responsible for any fetchMore-related logic. Still, I don't love the precedent that this simulated loading state sets, so I hope we can avoid similar hacks in the future.
benjamn
added a commit
that referenced
this issue
Jul 13, 2020
Should fix #6354, #6542, #6534, and #6459. Reliable delivery of loading results is one of the core benefits that Apollo Client strives to provide, especially since handling loading states tends to be such an annoying, error-prone task in hand-written state management code, and Apollo Client is all about keeping hand-written state management code to a minimum. Ever since I refactored the FetchPolicy system in #6221, the fetchMore method of ObservableQuery has not been delivering a loading state before sending its request. Instead, the observed query was updated only once, after the completion of the fetchMore network request. I've been aware of this problem for a while now, but I procrastinated solving it because I knew it would be, well, annoying. With the final release of AC3 right around the corner (Tuesday!), the time has come to get this right. This loading result doesn't fit neatly into the fetchQueryObservable system introduced by #6221, since the consumer of the loading result is the original ObservableQuery, but the network request gets made with a fresh query ID, using different variables (and possibly even a different query) passed to fetchMore. This separation is important so that we don't have to change the query/variables/options of the original ObservableQuery for the duration of the fetchMore request. Instead, the fetchMore request is a one-off, independent request that effectively bypasses the usual FetchPolicy system (technically, it always uses the no-cache FetchPolicy). In Apollo Client 2.x (and before #6221 was released in beta.46), this logic was guided by an extra fetchMoreForQueryId parameter passed to QueryManager#fetchQuery, which ended up appearing in a number (16) of different places, across three different methods of the QueryManager. I think it's an improvement that the logic is now confined to one block of code in ObservableQuery#fetchMore, which seems naturally responsible for any fetchMore-related logic. Still, I don't love the precedent that this simulated loading state sets, so I hope we can avoid similar hacks in the future.
benjamn
added a commit
that referenced
this issue
Jul 13, 2020
Should fix #6354, #6542, #6534, and #6459. Reliable delivery of loading results is one of the core benefits that Apollo Client strives to provide, especially since handling loading states tends to be such an annoying, error-prone task in hand-written state management code, and Apollo Client is all about keeping hand-written state management code to a minimum. Ever since I refactored the FetchPolicy system in #6221, the fetchMore method of ObservableQuery has not been delivering a loading state before sending its request. Instead, the observed query was updated only once, after the completion of the fetchMore network request. I've been aware of this problem for a while now, but I procrastinated solving it because I knew it would be, well, annoying. With the final release of AC3 right around the corner (Tuesday!), the time has come to get this right. This loading result doesn't fit neatly into the fetchQueryObservable system introduced by #6221, since the consumer of the loading result is the original ObservableQuery, but the network request gets made with a fresh query ID, using different variables (and possibly even a different query) passed to fetchMore. This separation is important so that we don't have to change the query/variables/options of the original ObservableQuery for the duration of the fetchMore request. Instead, the fetchMore request is a one-off, independent request that effectively bypasses the usual FetchPolicy system (technically, it always uses the no-cache FetchPolicy). In Apollo Client 2.x (and before #6221 was released in beta.46), this logic was guided by an extra fetchMoreForQueryId parameter passed to QueryManager#fetchQuery, which ended up appearing in a number (16) of different places, across three different methods of the QueryManager. I think it's an improvement that the logic is now confined to one block of code in ObservableQuery#fetchMore, which seems naturally responsible for any fetchMore-related logic.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
loadingvariable of useQuery result remains false whilefetchMorecall is pending network request. I havenotifyOnNetworkStatusChange: truein useQuery options.This seems to have been introduced in 3.0.0-beta.46 as it does not occur in 3.0.0-beta.45
Intended outcome:
expect
loadingvariable to be true after fetchMore is calledActual outcome:
loadingremains false afterfetchMoreis calledHow to reproduce the issue:
query with useQuery and paginate with
fetchMorenotice
loadingremains false afterfetchMoreis calledrevert back to version 3.0.0-beta.45 and do the same
notice
loadingis true afterfetchMoreis calledVersions
System:
OS: macOS Sierra 10.12.6
Binaries:
Node: 13.1.0 - /usr/local/bin/node
Yarn: 1.19.1 - /usr/local/bin/yarn
npm: 6.14.5 - /usr/local/bin/npm
Browsers:
Firefox: 59.0.1
Safari: 12.1.2
npmPackages:
@apollo/client: 3.0.0-beta.46 => 3.0.0-beta.46
The text was updated successfully, but these errors were encountered: