-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Throw an error if the link chain completes without emitting a value #12485
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
Conversation
🦋 Changeset detectedLatest commit: 9be41f9 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 |
|
commit: |
| obs.reobserve({ query, fetchPolicy: "standby" }) | ||
| // TODO: Update this behavior | ||
| ).rejects.toThrow(new EmptyError()); | ||
| ).resolves.toEqualStrictTyped({ data: undefined }); |
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.
I didn't add a changeset for this change since this was introduced with the RxJS switchover. I'd be happy to add one though if we'd like to point this out for the alpha release.
| queryId, | ||
| options, | ||
| networkStatus | ||
| ).observable.pipe(map(toQueryResult)), |
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.
Every area of the codebase that used fetchQuery converted the result to a QueryResult, so I refactored it to transform the value here instead.
| return observable; | ||
| } | ||
|
|
||
| // TODO: catch `EmptyError` and rethrow as network error if `complete` |
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.
Nice to get rid of these todos 👍
| // `value` can be `undefined` when the link chain only emits a complete | ||
| // event with no value which should be an error. | ||
| // https://github.com/apollographql/apollo-client/issues/12482 | ||
| if (!value) { |
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 was here to guard against the link chain completing without emitting a value. Now that it results in an error condition, this check is no longer needed.
| return lastValueFrom( | ||
| this.fetchObservableWithInfo(queryId, options, networkStatus).observable, | ||
| { defaultValue: undefined } | ||
| ) as TODO; |
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 TODO type felt great to get rid of 🙂
69db613 to
9a3957e
Compare
537d763 to
003daec
Compare
9a3957e to
271d756
Compare
1e3f7ed to
6a54982
Compare
59f6c06 to
07fd475
Compare
6a54982 to
a291b2b
Compare
07fd475 to
015d7d2
Compare
a291b2b to
4270fb9
Compare
015d7d2 to
915d25e
Compare
4270fb9 to
9b55005
Compare
915d25e to
482438f
Compare
9b55005 to
67430fd
Compare
36dc582 to
02d340e
Compare
67430fd to
c60325a
Compare
…ith standby fetch policy
c60325a to
9be41f9
Compare
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
size-limit report 📦
|
Closes #12482
Throws an error if a query or mutation does not emit a value before the link chain completes. This is usually a bug in the user's link chain and can have unintended consequences. By throwing, this makes the issue more visible and lets us clean up some areas of the codebase that would otherwise not be able to handle this situation.
This also addresses the issue where issuing a query with a
standbyfetch policy would throw anEmptyErrorand instead will resolve the query withdata: undefined.