Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schema extensions utility? #180

Closed
greyexpert opened this issue Oct 10, 2017 · 12 comments
Closed

Schema extensions utility? #180

greyexpert opened this issue Oct 10, 2017 · 12 comments

Comments

@greyexpert
Copy link

greyexpert commented Oct 10, 2017

Hello there,

Do you have any plans of porting extendSchema utility from graphql-js?
https://github.com/graphql/graphql-js/blob/master/src/utilities/extendSchema.js

It would be great to have ability to build modular schema and to extend type definitions using

extend type TypeName {}
@vladar
Copy link
Member

vladar commented Oct 11, 2017

That would be nice, but it's unlikely we'll port it soon (too little time now). So PRs are welcome!

@crirus
Copy link

crirus commented Oct 11, 2017

Hello guys,

I already have the code ported along with a merging/stitching function taken from Apollo. It's not 100 percent as they keep pushing more features but I can drop it on a repository so you can play with it and maybe help polishing the code

@greyexpert
Copy link
Author

It work this way in graphql-js at the moment:

The extend type statement is not processed when you build schema in graphql-js using buildASTSchema. The statement is processed only in extendSchema which accepts already built root schema and applies extensions to it.

What do you think, if we follow a slightly different approach:

We can add the support of extensions into BuildSchema itself without adding an extra utility like ExtendSchema. Having this feature supported, we will be able to build modular schema by just concatinating multiple schemas before passing them into BuildSchema::build

For example:

RootSchema.graphqls

schema {
  query: Query
}

type Query {
  version: String
}

UsersSchema.graphqls

type User {
  name: String
}

extend type Query {
  users: [User]
}

Then, all we need is to concatenate them into result schema and pass to BuildSchema::build

$schemaSources = [
    file_get_contents("RootSchema.graphqls"),
    file_get_contents("UsersSchema.graphqls")
];
        
$schema = BuildSchema::build(implode("\n\n", $schemaSources));

This feature will require less time, comparing to developing new utility. Also there are a lot of useful private methods in BuildSchema class, which can be re-used.

And, yes, I can create a pull request, if you guys don't mind this feature?

Related discussion in graphql-js repo:
graphql/graphql-js#922

@greyexpert
Copy link
Author

@crirus
It would be great if you share your implementation

@crirus
Copy link

crirus commented Oct 11, 2017

@greyexpert here is the initial code, any questions let me know
https://github.com/crirus/graphql-php-tools

@vladar
Copy link
Member

vladar commented Nov 22, 2018

@vladar vladar closed this as completed Nov 22, 2018
@MidnightDesign
Copy link
Contributor

MidnightDesign commented Dec 4, 2018

@vladar I'm sorry but I don't really get it. How is it supposed to be used? The class isn't referenced anywhere in the code and it has no public methods. Is it supposed to be extended? If so, why isn't it abstract and how is the inheriting class supposed to be used?

Edit: I'm an idiot. I didn't see the extend() method down there.

@MidnightDesign
Copy link
Contributor

@vladar I'm sorry if this is a dumb question, but is this supposed to somehow combine the two schema files

type Person {
    name: String!
}
type Person {
    age: Int!
}

into one

type Person {
    name: String!
    age: Int!
}

Because that's exactly what I would need.

@vladar
Copy link
Member

vladar commented Dec 5, 2018

@Torsten85 Can you please clarify as you are the one ported this utility?

@Torsten85
Copy link
Contributor

it should be used exactly like the graphql-js counterpart (https://github.com/graphql/graphql-js/blob/master/src/utilities/extendSchema.js#L103).

Small example:

$sdl = '
  type Query {
    defaultValue: String
  }
';

$documentNode = \GraphQL\Language\Parser::parse($sdl);
$schema = \GraphQL\Utils\BuildSchema::build($documentNode);

$extensionSdl = '
  extend type Query {
    extendendValue: String
  }
';

$extendedDocumentNode = \GraphQL\Language\Parser::parse($extensionSdl);
$extendedScheam = \GraphQL\Utils\SchemaExtender::extend($schema, $extendedDocumentNode);

echo \GraphQL\Utils\SchemaPrinter::doPrint($extendedScheam);

this will echo:

type Query {
  defaultValue: String
  extendendValue: String
}

@MidnightDesign
Copy link
Contributor

@Torsten85

Is the following supposed to work?

$sdl = '
  type Query {
    defaultValue: String
  }
  type Foo {
    value: Int
  }
';

$documentNode = \GraphQL\Language\Parser::parse($sdl);
$schema = \GraphQL\Utils\BuildSchema::build($documentNode);

$extensionSdl = '
  type Bar {
    foo: Foo
  }
';

$extendedDocumentNode = \GraphQL\Language\Parser::parse($extensionSdl);
$extendedScheam = \GraphQL\Utils\SchemaExtender::extend($schema, $extendedDocumentNode);

echo \GraphQL\Utils\SchemaPrinter::doPrint($extendedScheam);

I get a Uncaught Type "Bar" not found in document.. Obviously, this is because the extension schema references a type from the base schema, but what is this feature for if not for this use case?

@Torsten85
Copy link
Contributor

Torsten85 commented Dec 5, 2018

@MidnightDesign you are completely right, this should work! (https://codesandbox.io/s/yjmpnop8k9) I've digged a little deeper and found a "bug" in my implementation. This PR should fix this: #417

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

No branches or pull requests

5 participants