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

Subscriptions #32

Closed
D1plo1d opened this issue Apr 21, 2016 · 7 comments
Closed

Subscriptions #32

D1plo1d opened this issue Apr 21, 2016 · 7 comments

Comments

@D1plo1d
Copy link

D1plo1d commented Apr 21, 2016

Not sure how much work this is to implement but support for graphql-js compatible subscriptions (maybe with an experimental flag or something?) would be super-useful and really appreciated.

I (very foolishly) assumed libgraphqlparser would support subscriptions when we started building on top of it. Now we've got a whole ruby GraphQL DSL built and nowhere to subscribe :(

Related Conversations:

@swolchok
Copy link
Contributor

Does the attached commit work for you? Seems legit from the tests I added, but I would like to have other eyes on it before I merge it to master.

@D1plo1d
Copy link
Author

D1plo1d commented Apr 29, 2016

The graphql-parser gem seems to have an issue with this branch. I'm not a C++ person, any idea what's going on here:

Installing graphql-parser 0.0.2 with native extensions

Gem::Ext::BuildError: ERROR: Failed to build gem native extension.

    /Users/robgilson/.rbenv/versions/2.2.2/bin/ruby -r ./siteconf20160429-11555-1e20m4.rb extconf.rb
checking for main() in -lgraphqlparser... yes
creating Makefile

make "DESTDIR=" clean

make "DESTDIR="
compiling graphql_parser.c
compiling graphql_ruby.c
graphql_ruby.c:514:18: warning: implicit declaration of function 'GraphQLAstArrayValue_get_values_size' is invalid in C99 [-Wimplicit-function-declaration]
  return INT2FIX(GraphQLAstArrayValue_get_values_size(node));
                 ^
/Users/robgilson/.rbenv/versions/2.2.2/include/ruby-2.2.0/ruby/ruby.h:234:30: note: expanded from macro 'INT2FIX'
#define INT2FIX(i) (((VALUE)(i))<<1 | FIXNUM_FLAG)
                             ^
graphql_ruby.c:889:7: error: no member named 'visit_array_value' in 'struct GraphQLAstVisitorCallbacks'; did you mean 'visit_float_value'?
  cbs.visit_array_value = visit_array_value;
      ^~~~~~~~~~~~~~~~~
      visit_float_value
/usr/local/include/graphqlparser/c/GraphQLAstVisitor.h:38:26: note: 'visit_float_value' declared here
  FOR_EACH_CONCRETE_TYPE(FUNC_MEMBER)
                         ^
/usr/local/include/graphqlparser/c/GraphQLAstForEachConcreteType.h:21:28: note: expanded from macro 'FOR_EACH_CONCRETE_TYPE'
MACRO(IntValue, int_value) \
                           ^
/usr/local/include/graphqlparser/c/GraphQLAstVisitor.h:26:29: note: expanded from macro 'FUNC_MEMBER'
  visit_##snake_type##_func visit_##snake_type; \
                            ^
<scratch space>:68:1: note: expanded from here
visit_float_value
^
graphql_ruby.c:889:25: warning: incompatible pointer types assigning to 'visit_float_value_func' (aka 'int (*)(const struct GraphQLAstFloatValue *, void *)') from 'int (const struct GraphQLAstArrayValue *, void *)' [-Wincompatible-pointer-types]
  cbs.visit_array_value = visit_array_value;
                        ^ ~~~~~~~~~~~~~~~~~
graphql_ruby.c:890:7: error: no member named 'end_visit_array_value' in 'struct GraphQLAstVisitorCallbacks'; did you mean 'end_visit_float_value'?
  cbs.end_visit_array_value = end_visit_array_value;
      ^~~~~~~~~~~~~~~~~~~~~
      end_visit_float_value
/usr/local/include/graphqlparser/c/GraphQLAstVisitor.h:38:26: note: 'end_visit_float_value' declared here
  FOR_EACH_CONCRETE_TYPE(FUNC_MEMBER)
                         ^
/usr/local/include/graphqlparser/c/GraphQLAstForEachConcreteType.h:21:28: note: expanded from macro 'FOR_EACH_CONCRETE_TYPE'
MACRO(IntValue, int_value) \
                           ^
/usr/local/include/graphqlparser/c/GraphQLAstVisitor.h:27:33: note: expanded from macro 'FUNC_MEMBER'
  end_visit_##snake_type##_func end_visit_##snake_type;
                                ^
<scratch space>:71:1: note: expanded from here
end_visit_float_value
^
graphql_ruby.c:890:29: warning: incompatible pointer types assigning to 'end_visit_float_value_func' (aka 'void (*)(const struct GraphQLAstFloatValue *, void *)') from 'void (const struct GraphQLAstArrayValue *, void *)' [-Wincompatible-pointer-types]
  cbs.end_visit_array_value = end_visit_array_value;
                            ^ ~~~~~~~~~~~~~~~~~~~~~
3 warnings and 2 errors generated.
make: *** [graphql_ruby.o] Error 1

make failed, exit code 2

Gem files will remain installed in /Users/robgilson/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/graphql-parser-0.0.2 for inspection.
Results logged to /Users/robgilson/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/extensions/x86_64-darwin-15/2.2.0-static/graphql-parser-0.0.2/gem_make.out

@swolchok
Copy link
Contributor

Looks like the graphql-parser gem hasn't been updated to be compatible with this API-breaking change on master (which corrects an API difference from graphql-js): 5b83654

That's probably because I haven't cut a versioned release since making that change. I'll fix that, though I'd prefer to include subscription support rather than do a point release just for that.

To fix it in graphql-parser, you probably just need to change array_value to list_value.

@swolchok
Copy link
Contributor

I also had to change GraphQLAstArrayValue to GraphQLAstListValue locally. Here are the find/replace commands I used, in the graphql-parser directory:

perl -p -i -e 's|array_value|list_value|g' $(git grep -l array_value)
perl -p -i -e 's|ArrayValue|ListValue|g' $(git grep -l ArrayValue)

Hope this helps!

@D1plo1d
Copy link
Author

D1plo1d commented Apr 29, 2016

I've set up a fork of graphql-parser with your changes and added test for subscriptions but it isn't passing yet. Not sure why:

https://github.com/D1plo1d/graphql-parser/tree/feature/subscriptions

You can run the tests with: bundle install & rake compile & rake test

@D1plo1d
Copy link
Author

D1plo1d commented Apr 29, 2016

Nm, typos strike again! Totally works :)

@D1plo1d
Copy link
Author

D1plo1d commented Apr 30, 2016

w00t! Thanks @swolchok!

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

No branches or pull requests

2 participants