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

Load local GraphQL schema #3803

Merged
merged 4 commits into from Jul 19, 2021
Merged

Conversation

lodi-g
Copy link
Contributor

@lodi-g lodi-g commented Jul 11, 2021

I often work with GraphQL, and sometimes the introspection requests are blocked. In these case, the developers give me a GraphQL schema so I can still have the documentation.
However, Insomnia does not support importing local schema files, only fetching from internet. Therefore I must use Postman for this case, but I'd prefer to use Insomnia.

Closes #2743

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Looking good! I haven't tested with a local schema file yet, but code and UX looks good - submitting this review with one small piece of feedback.

image

Error message looks good too

image

I also pushed a commit to fix up how state is being used to avoid the as any 😄

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@dimitropoulos dimitropoulos left a comment

Choose a reason for hiding this comment

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

image

Works as advertised with https://raw.githubusercontent.com/apollographql/apollo-tooling/master/__fixtures__/misc/schema.json but I have a few questions.

In general, I think this is a bigger feature than we're making it out to be in terms of UX consideration (and this particular item is a good example of that). #3803 (comment) is a good example of that, but another one is: should there be any indication anywhere to a user that they're using a remote vs local schema? Right now there isn't one, right? I think their should be.

Don't get me wrong: I love this PR and I really like the use-case it solves, but I also want to be considerate to a good user experience when users are uploading local schemas.

@@ -374,6 +424,12 @@ class GraphQLEditor extends PureComponent<Props, State> {
);
Copy link
Contributor

Choose a reason for hiding this comment

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

(github won't let me quite select the line I want, but I'm talking here about _handleRefreshSchema).

As a user of this, the very first thing I did was change the schema on disk and hit "Refresh Schema":
image

And it just returns the error at the bottom "schema not yet fetched". I think if we're introducing a local schema option, we really need to handle this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this works the same way as proto files, as a one time import. There's no persistent link to a file on the file system (and I think this is good) such that refresh schema should work for local files.

@vercel vercel bot temporarily deployed to Preview July 19, 2021 21:41 Inactive
@develohpanda develohpanda enabled auto-merge (squash) July 19, 2021 21:41
@develohpanda
Copy link
Contributor

Thanks for the PR and contribution @lodi-g 😄 We're merging this for functionality, and we'll put in a follow-up PR to address some of the feedback above, as well as a slight improvement to the UX to discern between working with remote schemas (refresh and auto-fetch) and local schemas. 🙌🏽

@develohpanda develohpanda merged commit 7831682 into Kong:develop Jul 19, 2021
@lodi-g
Copy link
Contributor Author

lodi-g commented Jul 20, 2021

Thanks for accepting it @develohpanda. I unfortunately did not have the time to address your feedback but I wholeheartedly agree that the UX of my PR can be improved. Cheers! 🍻

dimitropoulos added a commit to dimitropoulos/insomnia that referenced this pull request Jul 26, 2021
dimitropoulos added a commit to dimitropoulos/insomnia that referenced this pull request Jul 26, 2021
dimitropoulos added a commit to dimitropoulos/insomnia that referenced this pull request Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for importing GraphQL schemas from local files.
4 participants