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

feat(subscriptions): added graphql-subscriptions support #340

Closed
wants to merge 3 commits into from

Conversation

Urigo
Copy link
Collaborator

@Urigo Urigo commented Feb 21, 2017

Adding subscriptions support with the subscriptions-transport-ws library.

Replaced dummy subscription with timeSubscription and publish it every second.

@asiandrummer
Copy link
Contributor

Hi @Urigo - I'm not sure if graphiql repo is the right place for this. What is the intention of this addition?

@leebyron
Copy link
Contributor

I agree with @asiandrummer - If subscriptions-transport-ws wishes to include test coverage that it works with GraphiQL, then you should add this sort of test to that repo, but I don't think it's appropriate for GraphiQL's test cases to be subscription-specific when the large majority of usage is not subscriptions and subscriptions are still in the early phase of RFC where changes are possible.

I'd prefer that we don't introduce volatility into GraphiQL's test harness until subscriptions are supported in a more first-class way, and that when we do so we don't upset the existing test coverage.

@Urigo
Copy link
Collaborator Author

Urigo commented Apr 11, 2017

@asiandrummer @leebyron sorry got the late response.
I totally agree that while we are in RFC process this should not be merged.
Also, I agree about making the transport not subscriptions specific and we are working on that (updates soon).

Maybe we could leave this PR open as kind of RFC like thing for GraphiQL and subscriptions?
While working on that a lot of questions came up, also in simple terms of the right UI for subscriptions (When new results come - should they override the previous ones or append? , when we fire new subscription request, should we close the previous?, UI indicator for open connection? etc...)

Anyway for people who want to use GraphiQL with subscriptions today, I wrote a blog post about it - https://dev-blog.apollodata.com/how-to-use-subscriptions-in-graphiql-1d6ab8dbd74b
and I will keep improving and iterating on that.

@asiandrummer
Copy link
Contributor

cc @robzhu for context

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@AGS-
Copy link
Member

AGS- commented Aug 23, 2018

Hey @Urigo, this seems to have been sitting here for a while and since we haven't really been able to come to a consensus on this, and I'm not sure what direction we're taking with this, I'm going to close this for now. Thanks for the contribution.

@AGS- AGS- closed this Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants