Fix nullability of GraphQL scalar types when passed as required arguments#2618
Conversation
…ents Currently, the generated rbis for a GraphQL mutation that contains an argument that uses a scalar type will always treat the argument as nullable, even when it is required. This PR fixes it so that required arguments are treated as non-nullable.
|
I have signed the CLA! |
| return_type = signature&.return_type | ||
|
|
||
| valid_return_type?(return_type) ? return_type.to_s : "T.untyped" | ||
| valid_return_type?(return_type) ? RBIHelper.as_non_nilable_type(return_type.to_s) : "T.untyped" |
There was a problem hiding this comment.
Can you give context on why we can assume coerce_input can't return nil?
There was a problem hiding this comment.
coerce_input has to handle both required and optional arguments, and for an optional argument it still gets called with an input of nil. So when we have a required argument the signature should no longer have a T.nilable(...) since we know the output won't be nil.
There was a problem hiding this comment.
Let's capture this in a comment since there's quite a bit going on, something like
# Wrap as non-nilable for required arguments. `coerce_input` supports both
# required and optional; optional arguments are re-wrapped below based on `type.non_null?`
There was a problem hiding this comment.
Great, I've updated it with the comment as suggested.
| valid_return_type?(return_type) ? return_type.to_s : "T.untyped" | ||
| # Wrap as non-nilable for required arguments. `coerce_input` supports both | ||
| # required and optional; optional arguments are re-wrapped below based on `type.non_null?` | ||
| valid_return_type?(return_type) ? RBIHelper.as_non_nilable_type(return_type.to_s) : "T.untyped" |
There was a problem hiding this comment.
I wonder if we should be using T::Utils.unwrap_nilable on the runtime type objects in cases like this, instead of String processing
Thoughts @KaanOzkan ?
3 participants
Motivation
Fixes #2337
Currently, the generated rbis for a GraphQL mutation that contains an argument that uses a scalar type will always treat the argument as nullable, even when it is required. This PR fixes it so that required arguments are treated as non-nullable.
Implementation
Updated
GraphQLTypeHelperto useRBIHelper.as_non_nilable_typein the current branch of logic.Tests
Added a new test in
spec/tapioca/dsl/compilers/graphql_mutation_spec.rb