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

Support displaying deprecated input fields in GraphiQL docs #1458

Conversation

romainletendart
Copy link
Contributor

GraphiQL supports displaying deprecated input fields through an opt-in configuration property.
This PR exposes a new GRAPHENE setting that is forwarded all the way through the aforementioned configuration property.

And deduplicate link declaration.
@romainletendart romainletendart force-pushed the rletendart/show-deprecated-input-in-graphiql-docs branch from f32f322 to 2767f41 Compare September 6, 2023 08:52
Copy link
Collaborator

@sjdemartini sjdemartini left a comment

Choose a reason for hiding this comment

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

Neat, thanks for adding this and updating the docs!

Do you happen to have a screenshot or something illustrating the effect of having this set to False (the default, and presumably the behavior today) and True? The docs from graphiql don't provide much info about inputValueDeprecation.

Set to ``True`` if you want GraphiQL to display a deprecation section on field doc view.

This setting is passed to ``inputValueDeprecation`` GraphiQL options, for details refer to GraphiQLDocs_.
Make sure your GraphQL server supports deprecation of input values before setting this to ``True``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused by this comment. Is this referring to the version of graphql-core (https://github.com/graphql-python/graphql-core), or graphene, or something else? In what situation would it not be supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sentence is actually wrong. I meant checking graphql-js version.

Where we could have a risk is when we would use a version of GraphiQL that would depend on an old version of graphql (i.e. graphql-js) that does not support input field deprecation, see: https://github.com/graphql/graphiql/blob/57b5fcd8749bd1c54f4a6a04a9ab07316fd05e71/packages/graphiql/resources/renderExample.js#L103

Only graphql>15.5 supports input field deprecation.

After checking, it seems anyway that graphene-django pins graphiql's version here: https://github.com/graphql-python/graphene-django/blob/main/graphene_django/views.py#L68 to 2.4.7 and this version of graphiql in turn uses graphql@16.5.0: https://github.com/graphql/graphiql/blob/graphiql%402.4.7/yarn.lock#L9991 which is above 15.5.

So I think I can just remove this warning as it should no longer be a problem.

``GRAPHIQL_INPUT_VALUE_DEPRECATION``
------------------------------------

Set to ``True`` if you want GraphiQL to display a deprecation section on field doc view.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I know what this means, but I'm not actually positive. I think it would be worth giving an example of a graphene/graphene-django schema definition for when this would come into play and what the effect of setting it to True is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added an example and some more details on what the setting actually does.

@romainletendart romainletendart force-pushed the rletendart/show-deprecated-input-in-graphiql-docs branch from 2767f41 to 6e7f63d Compare September 12, 2023 18:09
Copy link
Collaborator

@sjdemartini sjdemartini left a comment

Choose a reason for hiding this comment

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

Thank you for updating the docs, super helpful! I tested this end-to-end and it worked well for me. Nice addition! Just one super minor comment


class MyMutation(graphene.Mutation):
class Arguments:
input = types.MyMutationInputType()
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: types.MyMutationInputType() -> MyMutationInputType() since the class is defined immediately above in this example and not in a types module

@firaskafri firaskafri merged commit ee7560f into graphql-python:main Sep 13, 2023
@romainletendart romainletendart deleted the rletendart/show-deprecated-input-in-graphiql-docs branch September 13, 2023 09:40
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.

None yet

3 participants