Add headers support to GraphiQL #1016
Conversation
* add GRAPHIQL_HEADER_EDITOR_ENABLED setting with False default * use GRAPHIQL_HEADER_EDITOR_ENABLED to set headerEditorEnabled GraphiQL option * update graphQLFetcher and httpClient in static/graphiql.js to support custom query/mutation headers
|
@KubaJastrz I've applied your suggestions in 29f28bc |
radekwlsk
force-pushed
the
graphiql-headers
branch
3 times, most recently
from
d89f939
to
ca115f5
Compare
graphene_django/settings.py
Outdated
| # Set to True to enable GraphiQL headers editor tab | ||
| # This sets headerEditorEnabled GraphiQL option, for details go to | ||
| # https://github.com/graphql/graphiql/tree/main/packages/graphiql#options | ||
| "GRAPHIQL_HEADER_EDITOR_ENABLED": False, |
There was a problem hiding this comment.
Just curious, why not always have it enabled? I think the majority use case for GraphiQL in Django is going to be development, no? Having the header panel always enabled seems like it would be fine. If someone really wants to disable it, they can copy the template and configure GraphiQL themselves.
If we decide to keep it, you'll also need to update views.py to pass through the configuration option like we do with subscription_path as well as the template where it gets injected.
There was a problem hiding this comment.
I did not want to enable it by default to not change current behaviour. But I have no objections to make it True by default if that is desired.
I will have a look at views.py and template later today, thanks for info.
There was a problem hiding this comment.
@radekwlsk I love the focus on stability and definitely appreciate the forethought!
Given that GraphiQL is provided by graphene-django as a convenient developer tool outside of the critical path, I don't think it has the same requirements for interface stability as the rest of the library. We upgraded GraphiQL to the latest version very recently in 2.12 which changed the interface quite a bit, and had I known about this feature at the time, I would have just turned it on in that PR (no setting needed at all).
@jkimbo @mvanlonden any feelings on this? Turning it on (always) would make this a very small JS-only PR and one less setting to care about.
There was a problem hiding this comment.
@eabruzzese I've injected the setting to template. With enabling headers tab by default I will let a little bit more discussion to happen first.
There was a problem hiding this comment.
I'm fine with enabling the setting by default since it's purely an enhancement and it shouldn't break anything.
There was a problem hiding this comment.
This looks great thanks @radekwlsk !
As mentioned I think defaulting the headers setting to True is better.
@jkimbo Should I just flip the default setting to |
|
@radekwlsk I think having the setting is good. I would just default it to true |
Co-authored-by: Jonathan Kim <jkimbo@gmail.com>
Making use of graphql/graphiql#1543 this PR introduces ability to set custom headers in GraphiQL once
GRAPHIQL_HEADER_EDITOR_ENABLEDsetting is changed toTrue.This should resolve #850
Probably some docs for this functionality have to be added (and maybe a test).