gmac/graphql-stitching-ruby

Argument defaults are ignored during composition

Closed this issue · 1 comments

gmac commented

From @mikeharty:

I'm not totally clear on whether "default_value" is officially supported in the GraphQL gem, but we are using it and it does work. It appears to be left out of the implementation here: https://github.com/gmac/graphql-stitching-ruby/blob/main/lib/graphql/stitching/composer.rb#L369

I've worked around this adding a merge_default_values function into the monkey patch I mentioned above:

def merge_default_values(type_name, members_by_location, argument_name: nil, field_name: nil)
  default_values = members_by_location.values.map(&:default_value)
  
  return nil if default_values.any?(&:nil?)
  
  if default_values.uniq.length != 1
    path = [type_name, field_name, argument_name].compact.join('.')
    raise ComposerError, "Default values for `#{path}` must be the same."
  end
  
  default_values.first
end

and I updated the code at the point I linked to conditionally add the default_value via kwargs if it has a value, so that it doesn't default fields to nulls that previously had no default (caused validation issues otherwise)

type = merge_value_types(type_name, value_types, argument_name: argument_name, field_name: field_name)
default_value = merge_default_values(type_name, arguments_by_location, argument_name: argument_name, field_name: field_name)
kwargs = {}
kwargs[:default_value] = default_value unless default_value.nil? && type.non_null?
schema_argument = owner.argument(
  argument_name,
  description: merge_descriptions(type_name, arguments_by_location, argument_name: argument_name, 
                                                                    field_name: field_name),
  deprecation_reason: merge_deprecations(type_name, arguments_by_location, argument_name: argument_name, 
                                                                           field_name: field_name),
  type: GraphQL::Stitching::Util.unwrap_non_null(type),
  required: type.non_null?,
  camelize: false,
  **kwargs
)
gmac commented

Good call. You're right, default values were never considered. I guess the main question here is how should they work? Looks like you're nulling the default unless all schemas implement it. I'd be inclined to say it should work as:

  1. Any schema with a default value provides that default.
  2. If multiple schemas provide a default that do not match, then they get resolved by a value mediator.

Does that seem reasonable? Please post what you have there as a PR and I'd be happy to formalize this feature with you.