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

in gql default to subscription instead of query #122

Merged
merged 1 commit into from
Jun 7, 2021

Conversation

pie6k
Copy link
Collaborator

@pie6k pie6k commented Jun 4, 2021

All gql hooks default to subscribtion with .query being optional

@pie6k pie6k self-assigned this Jun 4, 2021
@linear
Copy link

linear bot commented Jun 4, 2021

@pie6k pie6k requested review from heikir, omarduarte and radzionc June 4, 2021 14:50
Copy link
Contributor

@heikir heikir left a comment

Choose a reason for hiding this comment

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

Looks good, but don't you think having subscriptions as the default is a bit misleading?

@pie6k
Copy link
Collaborator Author

pie6k commented Jun 4, 2021

my thinking was we want the app to work in realtime and I cought myself being confused a few times only because I forgot to add .subscribtion and something did not refresh nicely.

So I imagine in general we want it everywhere to see changes other people are doing in real time etc. Query will probably be rather unusual use case

Copy link
Contributor

@omarduarte omarduarte left a comment

Choose a reason for hiding this comment

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

I was a bit worried about our connections, but it seems like Hasura can handle this
https://github.com/hasura/graphql-engine/blob/master/architecture/live-queries.md

@omarduarte
Copy link
Contributor

Althou, take into account that we'll get into pitfalls like this:
apollographql/apollo-client#5267

@pie6k
Copy link
Collaborator Author

pie6k commented Jun 7, 2021

@omarduarte about the issue you posted. It'll not happen as our useSubscribtion hooks actually uses query hook as well to address exactly this issue.

    const [queryData] = useAsQuery(variables);
    const subscriptionResult = useRawSubscription(getSubscriptionQuery(), {
      ...options,
      variables: variables as Variables,
    });

    const data = useLatest(subscriptionResult.data, queryData);

👆 we get both query and subscribtion and useLatest. In case of optimistic update, only query will update and thus it'll become 'latest' resulting in correct hook result.

@pie6k pie6k merged commit d83b703 into master Jun 7, 2021
@pie6k pie6k deleted the aca-439-with-gql-default-to-subscription-instead branch June 7, 2021 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants