Graphql-js: Description when print a AST graphql object

[amian84]

Hi, currently I've parsing a graphQL schema with parse function to AST object. Later I split the schema into different array of object to store into different files, one per type (interface, object type, enums...).

When I print the partial AST object to string again, this function avoid the descriptions attribute.. Then I lost all schema description. There are any solution for that or maybe this will be implement as new feature?

Example:
type Object1{
#description of attribute att1
att1: String
}

When print the AST object I lost descriptions
type Object1{
att1: String
}

[IvanGoncharov]

@amian84 You are using legacy SDL syntax. You should use standard syntax for descriptions:

type Object1{
  """description of attribute att1"""
  att1: String
}

In that case, description would be preserved in print output.

[amian84]

Hi @IvanGoncharov , i'm trying with the standard syntax as you commented me. But the parser fails when trying parse the description

const graphqllang= require('graphql/language');
 var test= graphqllang.parse(`type Test{
        """Description"""
        a:String
    }`);

/home/damian/Projects/graphql-mocker/node_modules/graphql/language/parser.js:972
  throw (0, _error.syntaxError)(lexer.source, token.start, 'Expected ' + kind + ', found ' + (0, _lexer.getTokenDesc)(token));
  ^
GraphQLError: Syntax Error GraphQL request (2:9) Expected Name, found String
1: type Test{
2:         """Description"""
           ^
3:         a:String

[IvanGoncharov]

@amian84 It looks like you are using graphql-js pre 0.12.0.
Please run npm ls graphql and if it so update it.

[amian84]

@IvanGoncharov perfect, I updated and works perfect, now I've to face another problem... graphql-faker doesnt work with standard syntax for descriptions heheh

Thanks for all

[IvanGoncharov]

@amian84 It's tracked here: https://github.com/APIs-guru/graphql-faker/issues/41

[stephenh]

Well shoot, I'd been using # ... comments for ~year or so now and had never seen this other official syntax. That's cool.

Although fwiw we do have places in our SDL files that should be the new "quote-comments" (so I'm moving these comments over to those now) but we also have other places that are just commenting out "dead code" and/or making "notes to the maintainer" that aren't made as "javadocs/jsdocs" for the next node, i.e.:

type Foo { ... }

# old code
# asdf
# asdf

" Bar. "
type Bar { ... }

I'm trying to write a tool that loads .graphql files, makes a change to the AST, and saves the updated AST back to the original source file, i.e. similar to Jest's toMatchInlineSnapshot.

I have this tool basically working, but currently the # old code ... comments disappear from the output with print, which messes with the developer-experience i.e. "er, where did my comments go?".

For expediency, I'll probably just convert these # old code comments over to descriptions, but FWIW/IMO some of them are better left as "not associated to a type" comment nodes in the AST, that ideally would not be dropped and still output so that the parse + print cycle doesn't have any side-effects.

...just as more use-cases for old-/non-description comments, see in-SDL comments that apply to a set of nodes, i.e.:

extend type Query {
  # These next few queries are for our zaz module
  # with a longer description of the past rationale that
  # relates to both fooQuery and barQuery. I don't mean for
  # this to be pulled into the javadocs of `fooQuery` but I still
  # want it for my future maintainers to read.
  " fooQuery description. "
  fooQuery { ... }
  " barQuery description. "
  barQuery { ... }
}

I get "old comments are not the new descriptions", but it still seems like a mistake to drop them from the AST and break the isomorphism of print(parse(sdl)) === sdl.

If I wanted to fork graphql-js to keep them around just for us, does this seem doable or are there precedence/ambiguity/etc. reasons that led to them being handled this way?

[danielrearden]

@stephenh This is definitely something we want to add in the near future. Please see this issue for more context and a possible workaround.

[stephenh]

@danielrearden oh great! My fault for not seeing that newer issue. :-) Thanks for the quick response!