graphql-ruby icon indicating copy to clipboard operation
graphql-ruby copied to clipboard

default values for invisible arguments

Open eapache-opslevel opened this issue 11 months ago • 3 comments

If you have an argument with a default_value:, and the argument is not visible for some reason, then the key (and thus default value) is not even present in the arguments hash. This is logically correct, but has caught us in a bug a couple of time where somebody will write, e.g.

argument :foo, Boolean, required: false, default_value: true, public: false

def resolve(**args)
  if args[:foo]
    # ...
  end
end

And then the wrong branch will execute when somebody runs against the public schema (i.e. with foo not visible) because it will be nil and interpreted as false instead of the desired "default" of true.

The current behaviour is correct/coherent, and there are a couple of obvious ways around this (e.g. listing out all of the args as kwargs to the resolve method, which requires duplicating all the default values and is annoying). But I wanted to ask in case anybody has a better pattern for avoiding this, or if the description of the problem prompts a clever idea for a solution.

It may not be spec-compliant / probably causes just as many problems as it solves, but injecting the default value even for invisible arguments is kind of the most "obvious" solution here.

eapache-opslevel avatar Jan 02 '25 20:01 eapache-opslevel

Hey, yeah, I don't think it'd be good to change the current behavior from a compatibility standpoint.

I don't know of a good way of accomplishing this now apart from kwargs with duplicated default values, like you mentioned above (and this doesn't really count as a good way...).

A couple of other possibilities that come to mind:

  • Add some config like apply_default_when_hidden: true, which could be mixed in to your BaseField class so that it applied across the board.
  • Add a method like default_argument_values which returns a Ruby-style hash which could be merged into args to accomplish this (default_argument_values.merge(args) would apply defaults where no values were given by the client)

rmosolgo avatar Jan 08 '25 14:01 rmosolgo

Hum, yeah. What about making the args hash "strict" (is that a thing that ruby lets you do?) so that accessing a missing key at least raises instead of producing unexpected/undefined behaviour?

Also, I know @derek-etherton-opslevel / @Farjaad were bit by a variant of this issue recently as well, maybe they'll have opinions on how to best avoid it.

eapache-opslevel avatar Jan 14 '25 15:01 eapache-opslevel

"strict"

No, afaik Ruby just makes a plain hash out of **kwargs without any constrains about accessing keys.

Another edge-casey thing about the example above is that, since foo has required: false, the client may pass foo: null. In that case, args[:foo] would be nil, not true:

require "bundler/inline"

gemfile do
  gem "graphql", "2.4.10"
end

class MySchema < GraphQL::Schema
  class Query < GraphQL::Schema::Object
    field :echo, String do
      argument :foo, Boolean, required: false, default_value: true
    end

    def echo(**kwargs)
      kwargs[:foo].inspect
    end
  end

  query(Query)
end

pp MySchema.execute("{ echo(foo: null) }").to_h
# {"data" => {"echo" => "nil"}}

rmosolgo avatar Feb 21 '25 18:02 rmosolgo