Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

default values for invisible arguments #5198

Open
eapache-opslevel opened this issue Jan 2, 2025 · 3 comments
Open

default values for invisible arguments #5198

eapache-opslevel opened this issue Jan 2, 2025 · 3 comments

Comments

@eapache-opslevel
Copy link
Contributor

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.

@rmosolgo
Copy link
Owner

rmosolgo commented Jan 8, 2025

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)

@eapache-opslevel
Copy link
Contributor Author

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.

@rmosolgo
Copy link
Owner

"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"}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants