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

Support extensions in useSubscription #11854

Conversation

jcostello-atlassian
Copy link
Contributor

This adds optional support for extensions in useSubscription following the graphql-ws spec:

https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md

@apollo-cla
Copy link

@jcostello-atlassian: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

Copy link

netlify bot commented May 17, 2024

👷 Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 81bc23d

Copy link

changeset-bot bot commented May 17, 2024

🦋 Changeset detected

Latest commit: 81bc23d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@apollo/client Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jcostello-atlassian jcostello-atlassian force-pushed the jcostello2/subscription-extensions branch from 7a9d21a to 8b9bc7a Compare May 17, 2024 15:34
@phryneas
Copy link
Member

Thank you for the PR!

I'm gonna be honest, the two of us have the worst timing: I'm in the middle of rewriting that hook right now (you can see the current progress over here: https://github.com/apollographql/apollo-client/pull/11511/files#diff-aa77d592927baae676cedc8e3fda943bf63744e885b0328371c6ebd9d18aebdb) because previously it was breaking multiple rules of hooks and I can't find a minimal angle of fixing it.

Looking at this, I'm already happy that most of your changes are in the core. Phew 😅

I just wanted to let you know - we'll look at this, but before that we have to figure out #11511, I hope that's okay!

@jcostello-atlassian
Copy link
Contributor Author

Thanks for the heads up @phryneas! I'll close this for now and will raise an issue for future reference:

#11856

@phryneas
Copy link
Member

phryneas commented May 17, 2024

No, please keep it open - you put a lot of work into this and I believe we'll be able to salvage most of it.
Your contribution here is very welcome!

It just might take a while :)

@phryneas phryneas reopened this May 17, 2024
This adds optional support for extensions in useSubscription
following the graphql-ws spec:

https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md
@jcostello-atlassian jcostello-atlassian force-pushed the jcostello2/subscription-extensions branch from 8b9bc7a to 795f721 Compare June 13, 2024 21:06
@phryneas phryneas added this to the 3.11.0 milestone Jun 25, 2024
@jerelmiller jerelmiller changed the base branch from main to release-3.11 July 2, 2024 17:28
@phryneas
Copy link
Member

phryneas commented Jul 3, 2024

Hey @jcostello-atlassian!
It has been a while, but the https://github.com/apollographql/apollo-client/tree/release-3.11 branch now contains the new useSubscription implementation - could you please update this PR for that? :)

@phryneas phryneas requested a review from jerelmiller July 9, 2024 15:37
Copy link
Member

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

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

This looks great to me! Thanks for getting this added!!! 🔥

@jerelmiller jerelmiller merged commit 3812800 into apollographql:release-3.11 Jul 9, 2024
39 of 40 checks passed
@github-actions github-actions bot mentioned this pull request Jul 9, 2024
@github-actions github-actions bot mentioned this pull request Jul 22, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants