Validate that deprecated requirements aren't required #3137
Conversation
| @@ -164,6 +165,11 @@ def to_graphql | |||
|
|
|||
| def type=(new_type) | |||
| validate_input_type(new_type) | |||
| # This isn't true for LateBoundTypes, but we can assume those will | |||
| # be updated via this codepath later in schema setup. | |||
| if new_type.respond_to?(:non_null?) | |||
There was a problem hiding this comment.
Out of curiosity what's the expected relationship between new_type.non_null? and @null? Think we should validate that the two are consistent?
There was a problem hiding this comment.
I'm not sure... There are hacks that work like argument(name, type_string, required: false), where type_string may include !. For example, that's how parsing a schema from SDL works; The SDL loader code just passes the type string along as-is, with required: false:
graphql-ruby/lib/graphql/schema/build_from_definition.rb
Lines 261 to 262 in 91e7e34
As for @null, I consider it a private implementation detail of Argument. To find out if an argument's type is non-null, you can call argument.type.non_null? (with the exception noted here: GraphQL internal types don't have that available right away -- but by the time you run a query, argument.type.non_null? will work).
|
The dang integration test is failing, but 👍. I pushed a commit to cover types defined by strings. |
|
Thanks for the improvement! |
Following up on the discussion in #3133, this adds validation to prevent deprecating required arguments in the class based API. The validation added in #3015 only covered the legacy API.