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

Query normalization with useSubscription #188

Closed
imjasonmiller opened this issue Feb 3, 2023 · 1 comment
Closed

Query normalization with useSubscription #188

imjasonmiller opened this issue Feb 3, 2023 · 1 comment
Labels
🐛 bug Something isn't working

Comments

@imjasonmiller
Copy link
Contributor

imjasonmiller commented Feb 3, 2023

Hey, great library!

Perhaps I am holding it wrong, but I was caught by surprise by the documentation mentioning that queries parsed with gql from graphql-tag can be passed to useQuery, while this seems to work a bit different for useSubscription.

While the API reference states a DocumentNode can be passed as well, I received that exact DocumentNode within the subscriptionForwarder by setting it up as described by the guide. It looks like that will result in an error for graphql-ws's client as it takes a string and no normalization is performed, unlike useQuery.

Is this intended behavior or would it be possible to apply the normalization to query for useSubscription as well and/or expose the normalizeQuery function so it can be easily used within the subscriptionForwarder as well?

I'd gladly open a pull request, if so.

Thanks!

@logaretm logaretm added the 🐛 bug Something isn't working label Feb 8, 2023
@logaretm
Copy link
Owner

logaretm commented Feb 8, 2023

This is indeed a bug. I think it could be fixed inside handleSubscription.ts

something like this:

    const normalizedQuery = normalizeQuery(operation.query);
    if (!normalizedQuery) {
      throw new Error('A query must be provided.');
    }

    useResult(forward({ ...operation, query: normalizedQuery }) as any, true);

Feel free to PR, otherwise, I will PR it tomorrow since it looks like a serious bug for non apollo-server users and breaks compact with other protocol libs.

EDIT: Really wanted you to have a go at this but I think this is a bug that needs to be fixed ASAP, many thanks for pointing it out and love the way you explained it. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants