Graphql-js: buildASTSchema() should throw on encountering `extend` definition

Created on 20 Jun 2017  ·  11Comments  ·  Source: graphql/graphql-js

buildASTSchema() exists to parse a fully complete schema definition and produce a schema. However currently if it encounters an extend definition, or any other definition it does not use for that matter, it silently skips over it. This can lead to a confusing debugging situation.

Instead, buildASTSchema() should throw when it encounters a definition that it does not use, similar to how execute() throws if a the provided document contains any definition other than operations.

enhancement

Most helpful comment

Current implementation already fully support extensions using extendSchema function:

const { graphql, parse, buildSchema, extendSchema } = require('graphql');

let schema = buildSchema(`
  type Person {
    name: String!
  }

  type Query {
    person: Person
  }
`);

schema = extendSchema(schema, parse(`
  extend type Person {
    salary: Int
  }  
`));

var root = { person: () => ({ name: "John Doe", salary: 1234 })};

graphql(schema, '{ person {salary} }', root).then((response) => {
  console.log(response);
});

If it does work, what am I doing wrong in this example that I'm getting this error?

Thanks for example 👍
ATM, buildSchema silently ignore extensions and you need to use extendSchema for that.
This issue is about adding support for extensions in buildSchema.

All 11 comments

Out of curiosity: is there something that prevents supporting extend in buildASTSchema?

For example this definition:

type Queries {
  querySubset: QuerySubset
}

type Query {
  queries: Queries
}

extend type Queries {
  hello: String
}

Look semantically correct to me. When used in this form in may look a bit unusual, but the last definition could be loaded from a different file. From what I can tell, supporting extend does not have any downsides and provides more flexibility.

That would be new behavior, but I'd also be happy to entertain PRs adding that as well. If so, the same change should probably be made in extendSchema if multiple extend statements are found, and care should be put into ensuring that the order in which the extensions are applied result in the same output.

I think this might be a breaking change for graphql-tools. I believe the way it currently does schema extension concatenates a number of schema language strings together and then pulls the extensions out to add them in a second step.

I agree with @OlegIlyenko about looking for a solution that handles the extensions and perhaps warning on this case in the mean time.

Hey I would like to chip in on this one. Can I first take up the issue of adding a warning? Then I will try to understand and help implement handling extend statement?

Does extend work now? @leebyron
or do we expect them to work maybe in the future?

The upside I'm seeing is as such allowing people building graphql application to modularize their application schema.

Does extend work now?

@viztor Not at the moment but I plan to implement it for 14.1.0 release.
Currently, buildSchema/extendSchema is under heavy refactoring with the goal to implement #1389.

For usecase where somebody needs multiple files merge up to one AST I write this helper: https://gist.github.com/langpavel/9653b99afe993167fc4bacafbfcc7909
Feel free to use it as an example of implementation which merge extend types into defined ones. Of course WITHOUT ANY WARRANTY :-)

What it can do:

  • load multiple *.gql files
  • parse them
  • merge definitions into one Document
  • sort definitions by multiple criteria
  • print AST like concatenated SDL
  • merge extend nodes into definition nodes, so AST looks like extended FTW
  • you can print this AST into SDL file of course
  • you can run buildASTSchema on merged AST →
  • you can execute introspection query on it

really not much of code with this possibilities, look on gist :-)
BTW: I care about location and Sources

Hello, appreciate the work all of you do here. Just curious, when is the fix for this expected to be released?

@aishwar Thanks.
Can't say exact date but I will try to include it in 15.0.0 that I plan to release next month.

I've been flipping around 10+ issues on this topic, and I'd like some clarification: does Object extension actually work per the spec in the current version of this reference JS implementation, or is it undocumented because it only partially works (i.e. schema can be defined, but extended fields can't be queried)? (To further muddy the waters, extending types does work with graphql-tools.)

If it does work, what am I doing wrong in this example that I'm getting this error?

GraphQLError: Cannot query field "salary" on type "Person".

Current implementation already fully support extensions using extendSchema function:

const { graphql, parse, buildSchema, extendSchema } = require('graphql');

let schema = buildSchema(`
  type Person {
    name: String!
  }

  type Query {
    person: Person
  }
`);

schema = extendSchema(schema, parse(`
  extend type Person {
    salary: Int
  }  
`));

var root = { person: () => ({ name: "John Doe", salary: 1234 })};

graphql(schema, '{ person {salary} }', root).then((response) => {
  console.log(response);
});

If it does work, what am I doing wrong in this example that I'm getting this error?

Thanks for example 👍
ATM, buildSchema silently ignore extensions and you need to use extendSchema for that.
This issue is about adding support for extensions in buildSchema.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

galki picture galki  ·  3Comments

swist picture swist  ·  4Comments

ablbol picture ablbol  ·  4Comments

thomasdingemanse picture thomasdingemanse  ·  4Comments

pranshuchittora picture pranshuchittora  ·  3Comments