feat: skip option for subscription #1229
Conversation
👷 Deploy request for graphql-hooks-cra pending review.Visit the deploys page to approve it
|
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| @@ -22,6 +22,10 @@ function useSubscription< | |||
| const contextClient = useContext(ClientContext) | |||
| const client = options.client || contextClient | |||
|
|
|||
| if (options.skip) { | |||
| return | |||
There was a problem hiding this comment.
although tests are passing, I believe that returning early from a hook violates React's rules of hooks, specifically:
🔴 Do not call Hooks after a conditional return statement.
There was a problem hiding this comment.
Ah, you are correct. I have now moved the skip check inside of the useDeepCompareEffect.
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
simoneb
force-pushed
the
subscription-skip
branch
from
3f8c561 to
db10fdf
Compare
|
Can you please update the docs to document the option? https://github.com/nearform/graphql-hooks/?tab=readme-ov-file#useSubscription |
|
@simoneb Done |
README.md
Outdated
| @@ -419,11 +419,12 @@ To use subscription you can use either [subscriptions-transport-ws](https://gith | |||
|
|
|||
| `useSubscription(operation, callback)` | |||
|
|
|||
| - `operation`: Object - The GraphQL operation has the following properties: | |||
| - `options`: Object - The GraphQL operation has the following properties: | |||
There was a problem hiding this comment.
why did you rename this? for consistency, you may want to rename it above as well, and see if it's defined as "operation" elsewhere
There was a problem hiding this comment.
I renamed because it didn't feel accurate calling it operation as it represents more than just the gql operation now. It was a bit hasty though. I'll rename it back to operation.
There was a problem hiding this comment.
options is also fine, but let's keep it consistent!
There was a problem hiding this comment.
I reverted to operation since options is being used in other contexts and changing would require more adjustments.
|
@simoneb Will you be able to craft a release this week with the new option? |
|
@brianjd done |
|
Thanks @simoneb! |
What does this PR do?
Add skip option for subscriptions, making the creation of subscription conditional.
Related issues
Closes #1225
Checklist