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

Switch to interface{} for Subscribe() return #289

Merged
merged 1 commit into from
Nov 28, 2018

Conversation

matiasanaya
Copy link
Contributor

@matiasanaya matiasanaya commented Nov 13, 2018

Breaking change (subscriptions)

It's a lot easier to build graph-gophers/graphql-transport-ws around the interface{} type.

This PR is to make the changes in this other PR possible.

@matiasanaya
Copy link
Contributor Author

I really would prefer to return json.RawMessage but this library favours interface{} as far as I can tell. Thoughts?

@@ -19,14 +19,14 @@ import (
// If the context gets cancelled, the response channel will be closed and no
// further resolvers will be called. The context error will be returned as soon
// as possible (not immediately).
func (s *Schema) Subscribe(ctx context.Context, queryString string, operationName string, variables map[string]interface{}) (<-chan *Response, error) {
func (s *Schema) Subscribe(ctx context.Context, queryString string, operationName string, variables map[string]interface{}) (<-chan interface{}, error) {
Copy link
Member

Choose a reason for hiding this comment

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

What's the benefit of this change?

@pavelnikolov pavelnikolov merged commit 0079757 into graph-gophers:master Nov 28, 2018
@matiasanaya matiasanaya deleted the feature/subs-adapter branch November 29, 2018 05:11
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.

2 participants