-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Fix networkStatus
incorrectly reported as ready
when using errorPolicy: 'all'
with errors
#12362
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 613d2f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Docs preview has no changesThe preview was not built because there were no changes. Build ID: bc92e701cf27e9de5ac502d0 |
commit: |
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
size-limit report 📦
|
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
loading: true, | ||
networkStatus: NetworkStatus.loading, | ||
loading: false, | ||
networkStatus: NetworkStatus.error, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This networkStatus
might actually be legitimate since this is the loading state from the execute
call. Looks like we just hold onto the error
until the next request returns.
That said, its curious that we hold onto error
here. In other cases (such as refetch
) this error
is reset until the next result comes back. This is different since we are calling execute
again though which just re-evaluates the query against the fetch policy.
Should we be removing the error
on this result in this case, or should we continue to handle this as-is? I think the loading
state is the correct one here either way.
Fixes #12334
When calling
observableQuery.getCurrentResult()
when theerrorPolicy
was set toall
, thenetworkStatus
was incorrectly reporting the value asNetworkStatus.ready
when there were errors returned in the result. This has now been corrected to return asNetworkStatus.error
to match the value emitted from the observable.This bug also affected
useQuery
anduseLazyQuery
sinceuseQuery
usesgetCurrentResult
under the hood.