Skip to content
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 missing context option to useSubscription #7860

Merged
merged 3 commits into from
Mar 22, 2021

Conversation

jcreighton
Copy link
Contributor

Fixes #7824. This PR adds context to useSubscription's options.

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@jcreighton
Copy link
Contributor Author

@benjamn I noticed we're also not allowing errorPolicy to be passed into the options for useSubscription. Was that on purpose?

@jcreighton jcreighton requested a review from benjamn March 18, 2021 23:57
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me!

Before merging, let's also update the documentation. Specifically, I believe we need to add the context option to this <SubscriptionOptions/> MDX component, which gets rendered in the subscription docs and in the React Hooks API docs. If you push changes to this PR branch, Netlify should automatically render the docs so you don't have to build them locally.

@benjamn benjamn changed the title Fix context option omission in useSubscription Add missing context option to useSubscription Mar 22, 2021
@jcreighton jcreighton force-pushed the 7824-use-subscription-context branch from 979b4e5 to 6259fe9 Compare March 22, 2021 17:26
@jcreighton jcreighton force-pushed the 7824-use-subscription-context branch from 6259fe9 to f4e888e Compare March 22, 2021 19:43
@jcreighton jcreighton force-pushed the 7824-use-subscription-context branch from f4e888e to bd2e352 Compare March 22, 2021 19:51
@jcreighton jcreighton merged commit f5f1914 into main Mar 22, 2021
@jcreighton jcreighton deleted the 7824-use-subscription-context branch March 22, 2021 20:07
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useSubscription is missing context option
2 participants