First step towards GraphQL support #2822

Open
wants to merge 14 commits into
from

Projects

None yet

7 participants

@pvolok
Contributor
pvolok commented Nov 16, 2016 edited

This PR introduces basic functionality for GraphQL:

  • Schema parsing and loading
  • Validating queries (only fields and inline spreads for now)
  • Fields autocomplete
  • Type-at-pos for fields

TODO:

  • Tests
  • Figure out what's going on in gc_js.ml
  • Finish parser: directives, latest spec
  • type_normalizer.ml and type_visitor.ml

I would like to finish this PR before continuing on adding more functionality, as I might have to change the implementation substantially during the review process. I've never made so big PRs, so please tell me what info I have to provide here in order to simplify and speed up reviewing.

Also, any clues on when this PR might be reviewed would be great :)

Schema

Flow consumes schema in a GraphQL language. GraphQL config must be set in .flowconfig:

[options]
graphql_config=graphqlrc.json
// graphqlrc.json
[{
  "tag": "Relay.QL", // only ids or member expressions
  "schema": "schema.graphql",
  "regex": ".*/admin_app/.*" // optional
}]

Queries and fragments

Template strings tagged with a tag set in graphqlrc are parsed and validated.

This was referenced Nov 16, 2016
@kassens
Member
kassens commented Nov 16, 2016

Really excited to see this!

Looking at this from a Relay perspective. I'll need to dive into the generated types, but here's a first pass on features we probably need at Facebook:

  • Support multiple schema files: A mapping from directory or regex of the JS file to GraphQL schema would be great, for example you could have a different schema for the public and admin interfaces for example everything in src/js/admin would use admin.graphql whereas src/js/public uses public.graphql.
  • Make tags customizable: Instead of hardcoding Relay.QL and gql, it'd be nice for this to be the default and customizable in case we want to use something like RelayExperimental as a different tag, but same compiler handling.
@samwgoldman
Member

To make the tags customizable, I think a good approach would make use of a CustomFunT. If we set up Relay's Flow declarations to resolve Relay.QL to that CustomFunT, we could separate this logic from the hardcoded string.

I'll take a quick look to ensure that kind of approach is possible, because our tagged template support may be lacking some functionality to support it.

@josephsavona
Contributor

This is really exciting. There's enough code (and I'm unfamiliar enough with Ocaml) that it isn't super obvious what types are inferred. For example, the example file has:

const query = Relay.QL`
  query {
    me {
      id
    }
  }
`;

What is the inferred type for query in this case? (and using the example schema file) Ideally it would be something like:

var query: Query<{
  me: {
    id: string,
  },
}>;

type Query<D> = /* user defined, specified in config */;

i.e. Where the user gets to define the shape of the inferred type (probably with different inferred types for different template tags).

@samwgoldman
Member

Tests are going to be crucial here. I reached out to GraphQL folks internally and someone should follow-up with a bit of guidance on a good test plan for this work.

@asiandrummer

Very cool indeed :D Just throwing a couple of opinions from my end:

  • For the multiple schema case @kassens mentioned, we could use a configuration file (say .graphqlrc file) that includes the directory paths for files and the path for the corresponding schema. It's internally being used for setting up the GraphQL toolchain, and is discussed publicly to configure GraphQL environment.
  • I can almost see a graphql-ocaml package that could be extracted out, which by itself would be awesome ;) What do you think about creating another library that takes care of graphql implementation written in ocaml language, probably after we have it matured in here? Or am I misunderstanding something?
  • For the test plan, I think we could refer to how graphql-js repo does testing, and develop a similar test suite in ocaml environment, right?
@pvolok
Contributor
pvolok commented Nov 18, 2016

@kassens @asiandrummer Supporting multiple schema files and customizable tags is not very hard. Should I add them into this PR (even more code in already big PR)? I'm thinking about something like this:

# flowconfig
[options]
grpahql_config=graphqlrc.json

# graphqlrc.json
{
  "schemas": [{
    "files": "<path regexp>",
    "schema": "schema.graphql",
    "tag": "Relay.QL",
  }]
}

Or if you have an internal config format for that, I'd just conform to it.

@samwgoldman I think we want to parse fragments in statement.ml while traversing js ast. By the time when CustomFunT are handled, template string is already a StrT.


@josephsavona Relay.QLquery {...}`` is inferred as GraphqlOpT (operation). But this type might not contain enough information about the object shape. It may contain fragment spreads that cannot be resolved yet. In Apollo we can do this:

var frag = gql`
  fragment userInfo on User { ... }
`;
var query = gql`
  query {
    me { ...userInfo }
  }
`;
client.query({
  query,
  fragments: frag,
});

So the whole result object shape is unknown before we call client.query(). It would be cool to support this too.

Do we need to be able to annotate queries? Or this would be enough:

function fetch<T>(query: T): $ObjOfQuery<T> {...}

var query = Relay.QL`query { me { name } }`;

var data: {
  me: {name: string}
} = fetch(query);

@asiandrummer Actually I started making an ocaml-graphql library in parallel. But all query validation logic is build on flow-graph. Also, there is a WIP graphql implementation in OCaml: https://github.com/andreas/ocaml-graphql-server. But yeah, that will be cool, if we can extract any useful utilities later, we will see.


Regarding tests, I'll take a closer look at graphql-js and flow tests, and will start adding tests here.

@pvolok
Contributor
pvolok commented Nov 22, 2016

I checked graphql-js tests and ported all that test current functionality. I think this will be enough for query validation.

pvolok added some commits Oct 28, 2016
@pvolok pvolok [graphql] First step towards GraphQL support
- graphql language parser
- schema loading
- basic query/fragment validation without fragment resolving
- field autocomplete (on parsable schema)
- type-at-pos for fields
11b9b29
@pvolok pvolok [graphql] Fix ocp build. c6eeb92
@pvolok pvolok [graphql] Fix build with ocaml 4.01.0 2e1a836
@pvolok pvolok [graphql] Port fields-on-correct-type tests from graphql-js and fix e…
…rrors.
410887b
@pvolok pvolok [graphql] Builtin types. 94ed946
@pvolok pvolok [graphql] Port fragment-on-composite-types tests from graphql-js and …
…fix errors.
67b209c
@pvolok pvolok [graphql] Port scalar-leafs tests from graphql-js. 9396fa7
@pvolok pvolok [graphql] Partially port possible-fragment-spreads tests from graphql…
…-js.

Most test are commented out until fragment resolving is implemented.
df5e307
@pvolok pvolok [graphql] Finish the parser (directive definitions, null, locs everyw…
…here).
a6b9981
@pvolok pvolok [graphql] Visit tvars in gc_js.ml
a3a7e55
@pvolok
Contributor
pvolok commented Nov 23, 2016

If I understand it correctly, in gc_js.ml we want to mark all type variable reachable from any type. And I did just that. Didn't dig deep though.

pvolok added some commits Nov 24, 2016
@pvolok pvolok [graphql] Implement GraphQL types handling in type_normalizer and typ…
…e_visitor.
335d96b
@pvolok pvolok [graphql] Graphql config: multiple schemas, tag configuration. 45fa437
@pvolok
Contributor
pvolok commented Nov 29, 2016

Just in case if anyone was going to review this soon. I just realized that I greatly overused flow-graph. Flow-graph is only required to resolve fragments and convert query to object type. So I'm going to reimplement this.

@gabelevi
Contributor
gabelevi commented Dec 2, 2016

Alright, let's get this reviewed! I'll import this to get our internal tests running and I'll badger my co-workers into reading this!

@facebook-github-bot

@gabelevi has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@gabelevi
Contributor
gabelevi commented Dec 2, 2016 edited

Sorry, I missed your comment about reimplementing. I'll wait to bug people then!

@samwgoldman samwgoldman self-assigned this Dec 2, 2016
pvolok added some commits Dec 2, 2016
@pvolok pvolok [graphql] Commit before cleanup. e7739b4
@pvolok pvolok [graphql] Simpler schema acquiring.
7dcba48
@pvolok
Contributor
pvolok commented Dec 3, 2016

@gabelevi Now it's ready!

I also added settings customizing tags and settings different schemas based on file path regex.

There is also a draft for the GraphqlData<Fragment> type annotation, which is to be used for inferring object shape from graphql query. I haven't added any tests, leaving it to later work.

@samwgoldman
Member

Thanks, @pvolok! I'm excited to review this work :)

@pvolok
Contributor
pvolok commented Dec 5, 2016

I'm so happy! Feel free to ping me on IRC whenever you want to discuss anything (though I'm in a very different timezone).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment