Audit public interface to make sure it makes sense.

[lewisf]

This has been pulled from persistgraphql (https://github.com/apollographql/persistgraphql/issues/19) to be used as a standalone utility / tool. As such, we should audit the public interface to make sure it makes sense and make sure to be meaningful with what we expose.

[Poincare]

@lewisf

Couple of thoughts regarding the public interface and some minor implementation details.

• This file now contains a class but I'm not sure if this class is needed since the methods don't really operate on a single data structure. I think it would be fine to leave the methods accessible without a wrapping class, especially since all of the methods are static anyway.

• Maybe I am reading this file incorrectly, but it seems that the return value of each of these is a string, rather than a GraphQL AST. I guess whether or not we return strings or the AST depends on what this package is meant to do. If we decide, however, that the goal is not to touch the AST at all, then this file need not live in this project at all.

It seems to me that there are a few benefits to handling AST-level stuff in this package, such as:

1. It will make it easy to build tools like the in this issue
2. It will make the persistgraphql tool a very small layer on top of this package.

Thoughts?

[lewisf]

@Poincare thanks for the comments, they definitely help a lot.

• I'm not married to the idea of whether or not they should be classes or not. I'll move them out of the class.

Maybe it makes sense to return an AST to facilitate the building of some tools, but also allow just returning raw strings in cases where:

• an existing package parses query strings into AST's, in which case the value comes from pulling the query out of different source files
• an existing package only wants the raw string as written.

I'm thinking about apollo-codegen where right now it lacks the capability to extract from javascript source files and extract-gql could enable that. Eventually it may also make sense to move apollo-codegen to just use the AST's from extract-gql too.

I think allowing for both raw strings and ASTs doesn't muddy up the responsibilities of this package that much and allows it to provide more utility.

[Poincare]

Exposing both sets of methods (i.e. strings and AST) sounds great to me if it seems like just the strings would be useful as well.

[lewisf]

@Poincare cool. what might make sense in terms of an interface here?

new ExtractGQL(parseIntoAST: true)

or

const extractor = new ExtractGQL();
extractor.fromJS().toString()
extractor.fromJS().toAST()

or

import {
  ExtractGQLString,
  ExtractASTString,
} from 'extract-gql';

new ExtractGQLString()
new ExtractGQLAST()

I'll have to think about it a bit, but just laying out some options for discussion, thoughts?

[Poincare]

const extractor = new ExtractGQL();
extractor.fromJS().toString()
extractor.fromJS().toAST()

looks good to me as a concept. However, what if a particular JS file contains multiple queries? Would this just put them into a single string? I think it would probably be better to place them in a list.

[lewisf]

Good point, probably toStrings(), returning Array<string> makes more sense in that case.

[Poincare]

Right. I think toAST() should also return a list of ASTs rather than a single AST since we'd want to be able to differentiate between queries mentioned in different parts of the code.