Skip to content

Conversation

@IvanGoncharov
Copy link
Member

@IvanGoncharov IvanGoncharov commented Dec 29, 2017

Based on #1114 by @mjmahone

(await graphql(schema, introspectionQuery)).data is a pretty strange API that doesn't, IMO, match.

That is the specified definition of introspection. If we'd like a nominally easier to understand API, we can certainly add that, but it should be a one-liner implemented using exactly that line of code. I think #1115 could help make that easier by guaranteeing a sync result and at least avoiding async when it's not necessary

I think it's better to use execute instead of graphql since you are not spending time validation introspection query. But execute required parsed query so it's not an one-liner anymore.
Checking for errors is also a good practice even those introspection query should produce them.
And finally, you should manually cast the result to IntrospectionQuery type.

That's why I think we should have this wrapper function inside lib.

export function introspectionFromSchema(
schema: GraphQLSchema,
options: IntrospectionOptions,
): IntrospectionQuery {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note, it seems like the response type of this should be named IntrospectionResult

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leebyron It's pretty confusing name but it already defined here:

export type IntrospectionQuery = {|

Would be great to rename it to IntrospectionResult but I'm not sure how much code it will brake 😞

@leebyron leebyron merged commit 863fe2b into graphql:master Jan 8, 2018
@leebyron
Copy link
Contributor

leebyron commented Jan 8, 2018

Nice

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

3 participants