Document !type better or consider dropping it?

[burlesona]

Just some food for thought:

graphql-ruby/lib/graphql/define/non_null_with_bang.rb

Line 11 in 741f269

def !

which leads to:

field :id, !types.ID

I'm new to GraphQL Ruby, but have been developing in Ruby for almost ten years now. I've never seen the unary ! operator overloaded like this, and it jumped out at me on the project home page enough that I had to dive into the source code to figure out what the heck it did, finally discovering it just calls #to_non_null_type.

That's extremely surprising behavior. To me it seems like unnecessary syntax sugar that actually obfuscates the code rather than make it more readable.

It's not my library so I offer the following with no expectation that you'll want to make the change, but if I were maintaining this I'd drop the unary and adopt one of the following changes:

First preference, just shorten #to_non_null_type as #non_null and invoke it like:

field :id, types.ID.non_null

That's nice and short and it works exactly the way the code works now, while being extremely clear and readable.

Alternatively, if there are lots of these methods and invoking them as such feels clunky to you, or you don't want the hassle of making them chainable, you could add factory methods such that you can call the constant (subscript or parens depending on how you prefer to implement it).

field :id, types.ID[null: false]

or

field: id, types.ID(null: false)

Any of the above and I wouldn't have so much as raised an eyebrow going through the docs for GraphQL Ruby.

Again, I know it's not my code and I don't mean to come in and raise a fuss, just wanted to offer you some feedback which I hope you find constructive food for thought. If not, feel free to ignore :)

Meanwhile, thanks for writing a great OSS lib, I'll be off learning how the rest of it works 👍

[jorroll]

@burlesona, I assume the answer is because, as this is a library for working with graphql, much of the syntax and design is inspired by the graphql spec rather than ruby. Graphql-ruby's field :name, !types.String tries to resemble graphql's type syntax type Character { name: String! }.

As someone who found graphql-ruby after finding graphql, graphql-ruby's use of ! was quickly understandable (though I'll admit, I seem to remember my immediate interpretation was as a ruby not()).

This being said, I know the current dsl is something @rmosolgo is actively experimenting with / is interested in improving. I'm a fan of proposal #727 (now closed 😢 ) to move graphql type definitions into separate .graphql files. This choice would address your concern, as it would be obvious that the use of type Character { name: String! } was not ruby syntax.

[rmosolgo]

👍 thanks for opening an issue, it's also a source of heartburn for me, because in other code, I write if !type ... and it backfires 😖

I think we'll use null: instead in a future schema definition API.

[dkniffin]

This was also very confusing for me, learning this library for the first time. Everywhere else in almost all programming languages I've seen, a bang before a statement negates it. At the very least, it should be documented very clearly towards the beginning of documentation and guides.

[rmosolgo]

The new class-based API will use null: true|false instead of !. I'll release this new API in 1.8, and make it the default in 1.9. You can read the docs in advance here:

https://github.com/rmosolgo/graphql-ruby/blob/1.8-dev/guides/schema/class_based_api.md

[JoshCheek]

This was also very confusing for me, learning this library for the first time. Everywhere else in almost all programming languages I've seen, a bang before a statement negates it. At the very least, it should be documented very clearly towards the beginning of documentation and guides.

Fortran, man.

Anyway, I know doesn't matter anymore, but I figured I'd point out that you can move the unary operators to the back by calling them as methods. Woulda looked more like the query language and less like Ruby boolean logic.

class Type
  def initialize(**config)
    @config = { required: false, **config }
  end

  def !
    self.class.new **@config, required: true
  end

  Int = new name: 'Int'
end

Type::Int  # => #<Type:0x00007fcfe30368e8 @config={:required=>false, :name=>"Int"}>
Type::Int.! # => #<Type:0x00007fcfe30345c0 @config={:required=>true, :name=>"Int"}>