Require object types to have fields and input objects to have arguments

[rmosolgo]

The spec says that object types must have fields and that input object types must have arguments. This change adds that requirement to query execution and runtime. (Since Ruby classes are open, there's not a "compile-time" or "build-time" to check the definition.)

This change keeps the current behavior when all fields are hidden by .visible? checks -- the type is still accessible but all fields are hidden. It might be possible to raise an error in that case but I don't think it's worth it.

For backwards compatibility (or ... use cases ?), there's has_no_fields(true) and has_no_arguments(true). Adding these configs will silence errors from GraphQL-Ruby.

Also, RelayClassicMutation (and HasSingleInputArgument) will still create empty input objects by default, with has_no_arguments(true) called on them. This is how it has always worked and I don't want to 💥 existing users 😅

Fixes #4509

TODO

• Test errors raised when object has no fields
• Test errors raised when input object has no arguments
• Update tests for this constraint
• Consider warning or backwards compat option?
• Add a deprecation warning; raise error in 3.0

[gmac]

For backwards compatibility (or ... use cases ?)

FWIW: "use cases" is legitimate. We use ActiveSupport load hooks to assemble root scopes from across the codebase, which means root scopes start as empty object types. So, don't write has_no_fields off as useless!

[rmosolgo]

Thanks for sharing that, @gmac. I would expect this assertion to run during the first query that the schema executes, and by that time, any object type being used would have had its fields loaded. Is that right? Or did you find that this raised errors for you before those hooks ran?

If you did encounter errors, I'd be interested to see the backtrace. My goal is for this to halt on execution or dumping the schema, but for it to not halt on any boot-time steps.

[gmac]

I think this is more on us than GraphQL Ruby; our approach is pretty novel. Our mutation root now basically looks like this:

class MutationRoot < GraphQL::Schema::Object
  graphql_name "Mutation"

  if Rails.configuration.eager_load
    Hooks.run!
  else
    # allow no fields when running lazy load hooks
    has_no_fields(true)
  end
end

Then, across the codebase are load hooks that attach fields to an injected mutation object, ie:

Hooks.configure_mutation_root do |m|
  m.field :do_stuff, true
end

So basically, we have to load Mutation to inject it into load hooks. The new validations are fine when load hooks run eagerly within the object scope, but block when hooks run lazily from the externally-curated dev loading sequence. has_no_fields gave us what we needed to make this work in development environments, so please don't write it off as superfluous!

[rmosolgo]

Sure, I haven't heard of anyone using hooks to build the schema, but it should generally be supported without errors. Ruby classes being open, etc, everything is supposed to be open to extension at roughly any time. I'd be interested in seeing the backtrace on the error that this raised in your app, even just the GraphQL-Ruby parts, so I could get a sense for why it didn't work in this case.

Maybe it's because something in Schema.mutation(MutationRoot) looked up a field which raised this error?

The intent of the implementation here was to allow objects without fields until the object is used in a query. Because as described here, it's reasonably to have object definitions whose fields haven't been added yet. But I don't know of a use case for a object type with no fields at query time!

[gmac]

Oh, I see what you’re saying. I’ll turn it off and see where the error happened. It was specific to tests with mutations. Sounds like dev env runs JIT hooks as part of the query itself; which wouldn’t surprise me with the aggressive startup optimizations that a dedicated team is making across the codebase.