rmosolgo / graphql-ruby

Ruby implementation of GraphQL
http://graphql-ruby.org
MIT License
5.38k stars 1.39k forks source link

Provide a definition API that doesn't instance_eval #454

Closed rmosolgo closed 7 years ago

rmosolgo commented 7 years ago

.define hijacks the lexical scope, resulting in weirdness:

However, it makes an API that's pretty on paper.

Could we make an equally easy API for schema definition that doesn't require the bait-and-switch of instance_eval?

One point: types.String is the same as GraphQL::STRING_TYPE. One option would be to make those constants more user-friendly and suggest people to use them instead of types. helpers.

khamusa commented 7 years ago

DSLs are great, specially for readability, but as my schema grows I feel its advantages become less relevant compared to code reusability. I'm talking for example about using Ruby inheritance and mixins for defining types, objects and method calls instead of proc definitions. For instance, I've been seeing on my schema a myriad of input types that are very similar to other input types or object types and have been creating multiple type generators to avoid code duplication.

I hope I'm not hijacking this issue with a nonsense idea, and I'm pretty aware I'm not proposing anything concrete. I'm not even sure this isn't possible, but when designing on top of graphql-ruby I've been really missing a more bare-ruby approach. I hope I am not missing anything.

I'd be happy to know your thoughts on this.

rmosolgo commented 7 years ago

Not a hijack at all, that's just what I was hoping to get at in this issue!

Some ideas for input types in particular were mentioned in https://github.com/rmosolgo/graphql-ruby/issues/515.

I'm wary of using Ruby classes as types because of the risk of naming conflicts between library-defined methods and user-defined methods. Also, it seems a bit like a pattern mismatch to use a class because it would never be instantiated.

But, I agree with your point that we need better ways of reusing code. I recently saw a blog post that was looking for a good way to bind argument definitions to resolve functions too, definitely something we need!

rmosolgo commented 7 years ago

1.5.0 offers a big step forward in a totally different option for schema definition. Schema.from_definition accepts a resolve callable: https://github.com/rmosolgo/graphql-ruby/blob/master/spec/graphql/schema/build_from_definition_spec.rb#L770-L806

For example you could:

rmosolgo commented 7 years ago

I'm closing this as superseded by #727 or #820