Add support for GraphQL Schema Language #324

LegNeato · 2019-03-02T07:13:03Z

No description provided.

LegNeato · 2019-03-02T07:35:46Z

A couple of notes:

Naming is probably bad, please give suggestions if so.
I didn't document the graphql-parser-integration feature. Not sure if we want that as public API, though I currently made it public in case other projects need to use it.
I had a trait AsSchemaLanguage and implemented it for every MetaType. I didn't like how it made the code look with all the conditional compilation, so I centralized it into a SchemaTranslator instead.

theduke · 2019-03-05T11:43:36Z

I'm not so sure what the use case for this is.

The schema language is usually used the other way around AFAIK, as in bootstrapping a server from a schema file and adding resolver methods.

Downstream tools use a introspection query to get the schema, since that way the information is already structured in a standardized way.

Where do you see this being useful? The only one I could think of is generating a easily readable specification of your schema for human consumption.

LegNeato · 2019-03-05T15:42:13Z

You need a schema file to use https://github.com/graphql-rust/graphql-client among other use cases such as external tools. Sure, you could run the introspection query and that's the next thing I am adding. If the schema is defined in Juniper/rust, it is essentially "locked up" until runtime currently.

LegNeato · 2019-03-05T19:52:16Z

But now that I think about it, perhaps it makes sense for this to be its own little crate/project, similar to https://github.com/davidpdrsn/juniper-from-schema? An argument can be made that it should be part of juniper as well as not. Is it a view of the schema one would reasonably expect juniper to handle? Or is it a nice-to-have that isn't intrinsic to how juniper does things.

FWIW, Graphql.js has printSchema() built in, which does what this PR does (goes from internal language representation to IDL).

Then again, they also have a way to go from the IDL to the internal representation, which is what https://github.com/davidpdrsn/juniper-from-schema does in Rust land.

I guess the same arguments would apply to #307 as well? So you could argue that juniper should include both this PR and something like juniper-from-schema to match the capabilities of GraphQL.js, or you could argue that those can be handled by helper crates for people needing the functionality and juniper only focuses on the Rust definition and executing.

I'm going to open an issue.

codecov-io · 2019-03-08T22:41:43Z

Codecov Report

Merging #324 into master will decrease coverage by 0.22%.
The diff coverage is 72.64%.

@@            Coverage Diff             @@
##           master     #324      +/-   ##
==========================================
- Coverage   87.79%   87.57%   -0.23%     
==========================================
  Files         101      102       +1     
  Lines       14244    14457     +213     
==========================================
+ Hits        12506    12660     +154     
- Misses       1738     1797      +59

Impacted Files	Coverage Δ
juniper/src/lib.rs	`80.95% <ø> (ø)`	⬆️
juniper/src/schema/model.rs	`82.74% <58.53%> (-4.64%)`	⬇️
juniper/src/schema/translate/graphql_parser.rs	`74.68% <74.68%> (ø)`
juniper/src/schema/meta.rs	`88.7% <92.3%> (+0.13%)`	⬆️
juniper/src/types/scalars.rs	`87.42% <0%> (-0.56%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd6f192...caa7008. Read the comment docs.

theduke · 2019-03-09T07:34:33Z

First glance looks good, I'll review thoroughly later today.

One question is how much work it would be to fully switch over to graphql-parser instead of bringing it in as a feature.

Also maybe there is a more elegant way to detect builting types than __ prefix checking.

LegNeato · 2019-03-09T18:49:17Z

@theduke For switching entirely over to graphql-parser's AST I think the concerns in #138 (comment) are still valid?

Also, it would probably be pretty trivial to change this to go direct to the string output without going through graphql-parser. Not sure if we want to go that route though...

theduke · 2019-04-09T16:54:42Z

@LegNeato as a first step I have opened a PR that allows a borrowed AST on the parser: graphql-rust/graphql-parser#19

I'll investigate wheter we'd really want to switch to that AST, and compare it with our own.
But, mapping the AST from the parser to ours would be a very lightweight operation that shouldn't make a difference performance wise.

theduke · 2019-05-15T07:33:54Z

@LegNeato I'm working on switching to graphql-parser.

We could wait with this until that happens, or move ahead now if you'd like to get it in sooner.

theduke · 2019-05-15T07:43:12Z

juniper/src/schema/meta.rs

+impl<'a, S> Field<'a, S> {
+    /// Returns true if the type is built-in to GraphQL.
+    pub fn is_builtin(&self) -> bool {
+        // "used exclusively by GraphQL’s introspection system"


This seems a bit hacky, __ is not reserved so users could add their own objects/ fields/arguments/...

We could add a builtin: bool flag to the relevant schema types.

From https://github.com/graphql/graphql-spec/blob/master/spec/Section%203%20--%20Type%20System.md#schema:

"All types and directives defined within a schema must not have a name which begins with {"__"} (two underscores), as this is used exclusively by GraphQL's introspection system."

theduke · 2019-05-15T07:49:22Z

Mostly looks good.

Actually I'd be in favor of including it now, but without the feature, let's just build it in.

LegNeato · 2019-05-18T15:38:45Z

Ok, I'll rip out the features, rebase, and switch to borrowing after your patch lands.

arlyon · 2020-01-15T01:02:10Z

Juniper has changed quite a bit since August (especially with the recommendation to base new PRs on the async branch). That said, keen to test this PR out, I rebased this onto #493 and changed the graphql-parser dependency to depend on the master branch and (after sprinkling in some lifetimes) this works like a charm.

Seems like for this to be merged we are waiting on:

the async-await branch merge
a new version of graphql-parser

I can open a PR with my (minimal) changes, just know that I am still quite new to rust.

LegNeato · 2020-01-21T18:06:24Z

I updated master to use the async-await branch.

LegNeato · 2020-02-17T05:44:39Z

Would love a PR with the updated changes!

LegNeato · 2020-03-25T17:56:40Z

@arlyon any chance you can put up the rebased changes?

LegNeato · 2020-04-10T09:15:09Z

Friendly 👉@arlyon

arlyon · 2020-04-15T21:21:02Z

Hey, missed these for some reason. I was going to open a PR, but I noticed a new version of graphql-parser is out and it seems silly not to update. Lifetimes are a bit annoying to iron out so it's taking a bit of time.

Edit: thought I was close to done, but can't get it right so I'm giving up. There's clearly some lifetime stuff I don't quite grasp. Sorry for blocking.

LegNeato · 2020-04-16T07:12:27Z

@arlyon great, thank you! Yes, the new version should allow us to use &str instead of Strings and be a lot more memory efficient feel free to post WIPs if you get stuck!

LegNeato · 2020-06-06T03:45:19Z

This was landed as #676

LegNeato mentioned this pull request Mar 2, 2019

Integrate with Juniper? graphql-rust/graphql-client#20

Open

LegNeato force-pushed the schema branch from 3ac03c6 to c78424e Compare March 2, 2019 07:39

LegNeato added the enhancement Improvement of existing features or bugfix label Mar 2, 2019

LegNeato requested a review from theduke March 2, 2019 07:40

LegNeato force-pushed the schema branch 2 times, most recently from 45b7ecb to 2bed836 Compare March 2, 2019 17:46

LegNeato mentioned this pull request Mar 5, 2019

Determine if Juniper should support alternate representations #328

Open

Add support for GraphQL Schema Language

338960f

LegNeato force-pushed the schema branch from 2bed836 to 338960f Compare March 8, 2019 22:27

LegNeato added 6 commits March 8, 2019 14:53

Add an enum to schema language test

22f58ac

Add an interface to schema language test

ee767ce

Fix formatting

65cb4ca

Add a field to example interface

85554fc

Add a union to schema language test

1523914

Add a default argument and description to schema language test

caa7008

theduke reviewed May 15, 2019

View reviewed changes

LegNeato mentioned this pull request Aug 13, 2019

Generic AST graphql-rust/graphql-parser#19

Merged

davidpdrsn mentioned this pull request Feb 16, 2020

Merge schema language PR #3

Closed

LegNeato mentioned this pull request Mar 9, 2020

Is it possible to get Juniper and graphql-client working together? #550

Closed

arlyon mentioned this pull request Apr 17, 2020

Add support for GraphQL Schema Language #624

Closed

LegNeato mentioned this pull request Jun 5, 2020

Add support for GraphQL Schema Language #676

Merged

LegNeato closed this Jun 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for GraphQL Schema Language #324

Add support for GraphQL Schema Language #324

LegNeato commented Mar 2, 2019 •

edited

Loading

LegNeato commented Mar 2, 2019 •

edited

Loading

theduke commented Mar 5, 2019 •

edited

Loading

LegNeato commented Mar 5, 2019 •

edited

Loading

LegNeato commented Mar 5, 2019 •

edited

Loading

codecov-io commented Mar 8, 2019 •

edited

Loading

theduke commented Mar 9, 2019

LegNeato commented Mar 9, 2019 •

edited

Loading

theduke commented Apr 9, 2019 •

edited

Loading

theduke commented May 15, 2019

theduke May 15, 2019

LegNeato Mar 25, 2020

theduke commented May 15, 2019

LegNeato commented May 18, 2019

arlyon commented Jan 15, 2020 •

edited

Loading

LegNeato commented Jan 21, 2020

LegNeato commented Feb 17, 2020

LegNeato commented Mar 25, 2020

LegNeato commented Apr 10, 2020

arlyon commented Apr 15, 2020 •

edited

Loading

LegNeato commented Apr 16, 2020

LegNeato commented Jun 6, 2020

Add support for GraphQL Schema Language #324

Add support for GraphQL Schema Language #324

Conversation

LegNeato commented Mar 2, 2019 • edited Loading

LegNeato commented Mar 2, 2019 • edited Loading

theduke commented Mar 5, 2019 • edited Loading

LegNeato commented Mar 5, 2019 • edited Loading

LegNeato commented Mar 5, 2019 • edited Loading

codecov-io commented Mar 8, 2019 • edited Loading

Codecov Report

theduke commented Mar 9, 2019

LegNeato commented Mar 9, 2019 • edited Loading

theduke commented Apr 9, 2019 • edited Loading

theduke commented May 15, 2019

theduke May 15, 2019

Choose a reason for hiding this comment

LegNeato Mar 25, 2020

Choose a reason for hiding this comment

theduke commented May 15, 2019

LegNeato commented May 18, 2019

arlyon commented Jan 15, 2020 • edited Loading

LegNeato commented Jan 21, 2020

LegNeato commented Feb 17, 2020

LegNeato commented Mar 25, 2020

LegNeato commented Apr 10, 2020

arlyon commented Apr 15, 2020 • edited Loading

LegNeato commented Apr 16, 2020

LegNeato commented Jun 6, 2020

LegNeato commented Mar 2, 2019 •

edited

Loading

LegNeato commented Mar 2, 2019 •

edited

Loading

theduke commented Mar 5, 2019 •

edited

Loading

LegNeato commented Mar 5, 2019 •

edited

Loading

LegNeato commented Mar 5, 2019 •

edited

Loading

codecov-io commented Mar 8, 2019 •

edited

Loading

LegNeato commented Mar 9, 2019 •

edited

Loading

theduke commented Apr 9, 2019 •

edited

Loading

arlyon commented Jan 15, 2020 •

edited

Loading

arlyon commented Apr 15, 2020 •

edited

Loading