-
Notifications
You must be signed in to change notification settings - Fork 536
Subscription graphiql support #436
Subscription graphiql support #436
Conversation
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 [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
@@ -72,6 +82,7 @@ add "&raw" to the end of the URL within a browser. | |||
<script src="//cdn.jsdelivr.net/react/15.4.2/react.min.js"></script> | |||
<script src="//cdn.jsdelivr.net/react/15.4.2/react-dom.min.js"></script> | |||
<script src="//cdn.jsdelivr.net/npm/graphiql@${GRAPHIQL_VERSION}/graphiql.min.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally we should manage the react and other client-side libs via npm or bower, npm is recommended, then browserify and create bundle, in that way, some issue like #112 can be resolved.
@junminstorage Thanks for PR 👍 I think it's pretty big step to add Anyway, I think making such big decisions is beyond my current role in this project. @leebyron What do you think about this feature? |
@IvanGoncharov generally I would go with something working and keep iterating on it. Of course, that comment from their site might not be totally accurate - however, I think this is a good first step. Personally, I can put some time into this feature to improve and remove any dependency with some guidance of course. |
Is anything we can do to move this one forward? |
'subscriptions-transport-ws' is generic graphql subscription spec implementation, I am not sure why we need to reinvent a wheel. and this PR answered a well-liked community's request per original question from #390 (comment) |
It would be nice to see some movement on this PR |
ping @IvanGoncharov and @leebyron here. let us know what we need to do here to get it resolved. |
Hey guys, been working with this package, it would really be awesome if the subscriptions work so that we can start using this on our project. is there anything we can do to help speed up the process? |
Any updates on this? |
1 similar comment
Any updates on this? |
|
And now I'm going to re-open it, blushing heavily, because I just realised this is the Gosh, is my face red right now. |
thanks for pushing this through. Anyone from the team could provide us with an update on this please? keen to get subscriptions to work |
Hi All, I am original author for this PR. per the #436 (comment), the roadblocker is the dependency on subscriptions-transport-ws due to its repo was not under MIT license? Now this repo is under MIT now. Let me know if anything I can do, besides resolving the merge conflicts, to move it forward. |
that would be fantastic if you could do that @junminstorage ! going to add myself and daniel as reviewers so we can get this through. apologies the GraphiQL docs are so sparse on subscriptions, we need to add that to the docs/examples very badly. |
Awesome I am going to working on this PR this week. expect to have it ready by early next week. |
Looking forward to it! |
closed it and replaced by #686 |
replaced by #687 |
See #390 (comment) for usage