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

Possibly unnecessary Schema level resolve_type proc #1193

Closed
willcosgrove opened this issue Dec 15, 2017 · 4 comments
Closed

Possibly unnecessary Schema level resolve_type proc #1193

willcosgrove opened this issue Dec 15, 2017 · 4 comments

Comments

@willcosgrove
Copy link

Hi! I'm not sure if this is a bug, or just a case where I didn't understand the documentation properly but this is a problem I've been running into any time I use union types, though I think it would also apply for interfaces.

When I add a union type to my schema, it fails to validate until I add a resolve_type proc to the schema definition. I would have expected that to be unnecessary as long as I defined a resolve_type proc on the union type itself, which I did. Here's a quick failing ruby script:

require 'graphql'

module This
  extend self

  def hello
    "Hi, from This!"
  end
end

module That
  extend self

  def hello
    "Bonjour from That!"
  end
end

ThisType = GraphQL::ObjectType.define do
  name "This"
  field :hello, !types.String
end

ThatType = GraphQL::ObjectType.define do
  name "That"
  field :hello, !types.String
end

ThisOrThatType = GraphQL::UnionType.define do
  name "ThisOrThat"
  possible_types [-> { ThisType }, -> { ThatType }]
  resolve_type -> (obj, _ctx) {
    case obj
    when This then ThisType
    when That then ThatType
    end
  }
end

QueryType = GraphQL::ObjectType.define do
  name "Query"
  field :thisOrThat, -> { ThisOrThatType } do
    resolve -> (_, _, _) {
      [This, That].sample
    }
  end
end

Schema = GraphQL::Schema.define do
  query QueryType
  # resolve_type -> (_,_,_) { } # uncomment this line and the schema will validate
end

From what I can tell, the docs around the union type feature don't say that you have to define a resolve_type proc on schema, but that a resolve_type proc must be defined either on the schema, or on the union type.

It seems to me like the schema should validate without the resolve_type proc being defined at the schema level, especially since it appears to function fine when you stub it with a no-op proc.

Here's a query for testing out the above schema:

{
  thisOrThat {
    ... on This { hello }
    ... on That { hello }
  }
}
@rmosolgo
Copy link
Owner

Ah, I think you're right, the top-level function should not be required when all abstract members have local functions! Thanks for the detailed description :)

@cantino
Copy link

cantino commented Jan 1, 2018

I'm having this problem as well.

@jturkel
Copy link
Contributor

jturkel commented Mar 13, 2018

FWIW - This appears to be fixed in 1.8-dev (as of commit c618943) if the test case is updated to use the new class based API:

module This
  extend self

  def hello
    "Hi, from This!"
  end
end

module That
  extend self

  def hello
    "Bonjour from That!"
  end
end

class ThisType < GraphQL::Schema::Object
  field :hello, String, null: false
end

class ThatType < GraphQL::Schema::Object
  field :hello, String, null: false
end

class ThisOrThatType < GraphQL::Schema::Union
  possible_types ThisType, ThatType
  def self.resolve_type(obj, _ctx)
    case obj
    when This then ThisType
    when That then ThatType
    end
  end
end

class QueryType < GraphQL::Schema::Object
  field :thisOrThat, ThisOrThatType, null: false,
        resolve: -> (_, _, _) {
          [This, That].sample
        }
end

class Schema < GraphQL::Schema
  query QueryType
end

The problem still reproduces in 1.8-dev when using the DSL though.

@rmosolgo
Copy link
Owner

That's a nice side-benefit, thanks for updating us!

I'm going to close this because:

  • It's fixed in the class-based API
  • and, there's a straightforward work-around for the time-being.

Feel free to reopen if you find trouble with either of those!

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

4 participants