Add onError option to useSubscription hook #9495
Conversation
|
Just commenting to demonstrate my support and that I would love to see this feature added 🙏🏼 |
jeroenvisser101
force-pushed
the
jv-use-subscription-on-error
branch
from
71301fd
to
0084d9f
Compare
|
@trevorblades I'm sorry to ping you directly, but can you point me to the correct way (or person to reach out to) of getting this PR reviewed and merged? (I've just updated the branch so it's in-sync with |
jeroenvisser101
force-pushed
the
jv-use-subscription-on-error
branch
from
0084d9f
to
ad85057
Compare
jpvajda
added
the
🏓 awaiting-contributor-response
benjamn
removed
the
🏓 awaiting-contributor-response
There was a problem hiding this comment.
LGTM with one question (below). Thanks for the implementation and especially the tests @jeroenvisser101!
| onSubscriptionData?: (options: OnSubscriptionDataOptions<TData>) => any; | ||
| onError?: (error: ApolloError) => void; | ||
| onSubscriptionComplete?: () => void; |
There was a problem hiding this comment.
How would you feel about calling this onSubscriptionError, for consistency with the other onSubscription* options? I like onError better too (for brevity), but to my eyes it looks a little out of place here.
There was a problem hiding this comment.
I like onError more as well, but we can't change the others in 3.7 so I'm 👍 with making it onSubscriptionError.
There was a problem hiding this comment.
@benjamn confirmed we can release this change, but we want to consider a future improvement to change on
onSubscriptionData and onSubscriptionComplete to onData and onComplete
|
I think I'm going to merge this into |
Feature
Adds
onErroroption to theuseSubscriptionhook, similar to how it is offered on other hooks.I've added a simple test, but wasn't able to add an error-only result, I had to send
dataas well, I'm not sure why. If there's a better way to do that, I'd love to learn.Related issues and feature requests: