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

Add 'introspectionFromSchema' utility function #1187

Merged
merged 1 commit into from
Jan 8, 2018

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants