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

Provide a definition API that doesn't instance_eval #454

Closed
rmosolgo opened this issue Dec 11, 2016 · 4 comments
Closed

Provide a definition API that doesn't instance_eval #454

rmosolgo opened this issue Dec 11, 2016 · 4 comments

Comments

@rmosolgo
Copy link
Owner

.define hijacks the lexical scope, resulting in weirdness:

  • Loss of outer self, uh, "obviously"
  • Confusing NoMethodError if you make a typo when referencing a local variable

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
Copy link
Contributor

khamusa commented Feb 7, 2017

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
Copy link
Owner Author

rmosolgo commented Feb 7, 2017

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 #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
Copy link
Owner Author

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:

  • Describe a naming convention for Ruby constants

  • Accept a definition string

  • Wrap the schema definition in a custom method like

    module MagicGraphQL
      # Build a schema with a user-provided definition string 
      # which finds Ruby constants and uses them for field resolution
      def self.build_schema(definition_string)
        schema = GraphQL::Schema.from_definition(
          definition_string, 
          default_resolve: ResolveByConvention
        )
        # Have to satisfy some schema-level requirements:
        schema.redefine(
          object_from_id: ObjectFromIdByConvention,
          id_from_object: IdFromObjectByConvention,
          resolve_type: ResolveTypeByConvention,
        )
      end 
    end 

@rmosolgo
Copy link
Owner Author

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

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