Preserve @deprecated when reason = null #4006
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed an issue where creating a
GraphQLSchema
from an SDL and then printing it again results in dropping a@deprecated
directive if the reason is explicitly set tonull
.In
GraphQLSchema
we don't store the existence of a deprecated directive and its reason separately. We simply store thedeprecatedReason
. Becausereason
has a default, this is mostly fine since even if you don't provide a reason string, you still end up with one. However, the oddities of defaults in GraphQL means that you can pass an explicitnull
as your reason.In this case you end up with
deprecatedReason: null
in yourGraphQLSchema
which we currently treat as the same asundefined
when printing, meaning the@deprecated(reason: null)
gets dropped.I open this PR mostly to raise this issue rather than to advocate for this specific solution. I recognize that interfaces like
GraphQLFieldConfig
are part of the public API, and thus this might mean the change is technically a breaking change, since there are likely places where people assumed they could setdeprecatedReason: null
to indicate the field was not deprecated.Another (even more breaking) option would be to decide that the reason argument to
@deprecated
should be non-optional, and thus allow us to always assume that every deprecated field has a non-nullable reason.