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 GraphQL Schema Language? #241

Closed
brimworks opened this issue Nov 16, 2016 · 24 comments
Closed

Support GraphQL Schema Language? #241

brimworks opened this issue Nov 16, 2016 · 24 comments

Comments

@brimworks
Copy link
Contributor

The documentation for GraphQL uses a schema language that apparently is still a little in flux given this pull request:

graphql/graphql-spec#90

...and some tooling code is already being created around the schema language:

https://github.com/apollostack/graphql-tools

If I was to send a pull request to support the specification in the above pull request would it have a chance of being accepted?

Thanks,
-Brian

@dminkovsky
Copy link
Contributor

Hi @brimworks, thanks for posting.

I would say yeah, definitely, to the the extent that the schema language is supported by the JS implementation. I'm not sure what that extent is, but it seems to be parsing.

@brimworks
Copy link
Contributor Author

Thanks @dminkovsky for the green light. I'll be working on this and posting a PR (hopefully by the end of this week).

Thanks,
-Brian

@dminkovsky
Copy link
Contributor

Cool, thanks Brian. Post if you have any questions. Are you familiar with Spock?

@brimworks
Copy link
Contributor Author

Not to familiar with Spock, but I'm adding tests to ParserTest.groovy and it seems pretty straight forward. So far I've added to the ANTLR grammar the new syntax, then modified GraphqlAntlrToLanguage.java to generate the new "language" nodes of the AST. I can push my work in progress if you want to start the review process early.

@dminkovsky
Copy link
Contributor

Cool sounds great. No, please take your time :). Thank you.

@dminkovsky
Copy link
Contributor

By the way, a related issue if you have any time and interest is #126. I think it just needs tests ported-adapted from the JS implementation and any bug fixes that result. Would really bolster the tooling.

@brimworks
Copy link
Contributor Author

I quickly skimmed that issue, but it was unclear to me what format the printer prints in. I'm ASSUMING it iterates over the AST and pretty prints the original input in GraphQL syntax. If that is the case, then all these new AST symbols I'm adding will have a direct impact on the pretty printer (since there will be a host of new symbols to handle).

Unfortunately, the work I'm doing currently doesn't require such a pretty printer, but I do see value in it. So, I probably won't have time to contribute to that effort in the near future.

@dminkovsky
Copy link
Contributor

Cool, thank you. Yeah, that code will have to be updated to accommodate the
AST symbols you're adding. It won't be anything.

ср, 16 нояб. 2016 г. в 17:42, Brian Maher notifications@github.com:

I quickly skimmed that issue, but it was unclear to me what format the
printer prints in. I'm ASSUMING it iterates over the AST and pretty prints
the original input in GraphQL syntax. If that is the case, then all these
new AST symbols I'm adding will have a direct impact on the pretty printer
(since there will be a host of new symbols to handle).

Unfortunately, the work I'm doing currently doesn't require such a pretty
printer, but I do see value in it. So, I probably won't have time to
contribute to that effort in the near future.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#241 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AANWZb50dSseQH1KsFZ7BfakJ1qA1N6Nks5q-4bpgaJpZM4K0Dv1
.

@brimworks
Copy link
Contributor Author

This was easier than I thought... so I've posted this pull request: #243

Feedback is welcomed.

@brimworks
Copy link
Contributor Author

I realized today that I forgot to add the 'schema', 'extend', and 'directive' type declarations, so the latest commit adds support for those.

@dminkovsky
Copy link
Contributor

Cool, thanks very much. I'll take a look today or tomorrow.

@brimworks
Copy link
Contributor Author

Thanks for reviewing this!

@danielFesenmeyer
Copy link

This is a really nice feature :) Are there any plans when this will be released?

@brimworks
Copy link
Contributor Author

I'm waiting for feedback from @dminkovsky . I've got a code generator that uses this feature that I'd like to release, but it currently depends on my fork. I was hoping to get this merged and released so I don't have to depend on a fork.

Thanks,
-Brian

@dminkovsky
Copy link
Contributor

dminkovsky commented Dec 14, 2016 via email

@brimworks
Copy link
Contributor Author

BTW @dminkovsky here is the code generator I wrote that uses this code:

https://github.com/Distelli/graphql-apigen

Thanks,
-Brian

@dminkovsky
Copy link
Contributor

Brian, thank so much for updating here. This looks really cool. I haven't had a chance to try this and likely won't for some time, but I am really interested in this approach and look forward to any updates as you make progress.

Best, and happy 2017,
Dmitry

@brimworks
Copy link
Contributor Author

Thanks for the encouragement! I wish the best for you and your family in 2017.

The idea for this "schema first" development is partially from this JS project:

https://github.com/apollostack/graphql-tools

In fact my example uses the same Author/Posts example ;).

@dminkovsky
Copy link
Contributor

Thanks Brian, to yours as well!

I see the example uses the BatchedExecutionStrategy. How's that working for you?

By the way, are you looking to make this into a product?

@brimworks
Copy link
Contributor Author

The BatchedExecutionStrategy seems to work fine, but I'll admit that I haven't done a ton of testing with it.

I'm not currently think of turning this into a product, but if you have ideas toward that end, I would be interested :).

Thanks,
-Brian

@dminkovsky
Copy link
Contributor

dminkovsky commented Jan 8, 2017

The BatchedExecutionStrategy seems to work fine, but I'll admit that I haven't done a ton of testing with it.

Cool. I've not played with it and it predates my time with this project. So, just wondering.

I'm not currently think of turning this into a product, but if you have ideas toward that end, I would be interested :).

Yeah, me too! I'm not sure what one could do with this beyond something like reindex.io or apollo. I also came across this profile the other day, but the whole consulting/evangelist thing isn't very appealing to me (not related to apigen, either).

@industrialist
Copy link

Hi Team,
Nice project! I'm keen to understand where this has ended up as I'm interested in extending our existing graphQL schema to a new Spring-based microservice we're adding to our project. I'm just a little unclear as to where things are up to with this. Let me know if I'm being a noob?
Cheers

@brimworks
Copy link
Contributor Author

graphql-java supports parsing GraphQL schema language. So far I know of two projects that use this:

https://github.com/Distelli/graphql-apigen

...which generates interfaces and wiring code that allow you to simply implement the interfaces and wire-up a GraphQL object that you can query/mutate with.

https://github.com/Cox-Automotive/graphql-java-tools

...which appears to adapt existing objects so they conform to a GraphQL schema (at least that is what I grasp from a casual reading of the README).

@industrialist
Copy link

Thank you!

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

4 participants