Lookahead#selects? not matching fragment/type fields

[RobinDaugherty]

There seems to be an issue with fields that are selected through fragments or types of a field. While the selections method returns the expected list of field names, calling selects? does not return true.

For example (extracted from a spec) I need know if the name field was requested on either the BirdSpecies or BirdGenus type.

query = GraphQL::Query.new(LookaheadTest::Schema, <<-GRAPHQL)
  query {
    node(id: "Cardinal") {
      ... on BirdSpecies {
        name
      }
      ... on BirdGenus {
        name
      }
      id
    }
  }
GRAPHQL

node_lookahead = query.lookahead.selection("node")

node_lookahead.selections.map(&:name)
=> [:id, :name, :name]

node_lookahead.selects?('id')
=> true

node_lookahead.selects?('name')
=> false

node_lookahead.selects?('BirdSpecies.name')
=> false

node_lookahead.selects?('birdSpecies.name')
=> false

I've added specs for Lookahead#selects? to demonstrate this. Am I correct that this is a bug, or am I misusing the selects? method?

[rmosolgo]

Hey, yes, you've understood correctly, it's a bug! Do you want to take a crack at fixing it in this PR?

[timward60]

I believe this is because the way selection is implemented (which is called by selects?):
def selection(field_name, selected_type: @selected_type, arguments: nil)

selected_type is used to find the corresponding field selections. In the case of queries above the selected_type will be "Node". You can get the behavior desired above via:

node_lookahead.selection(:name, selected_type: schema.find_type("BirdGenus")).selected?
node_lookahead.selection(:name, selected_type: schema.find_type("BirdSpecies")).selected?

[gsdean]

Coming from here: https://github.com/DmitryTsepelev/graphql-ruby-fragment_cache#limitations

Is there a proposed solution? We'd be willing to work on a PR, but not exactly sure where to start

[rmosolgo]

Hey @gsdean , this branch contains example tests but not a fix. Yes, if you want to open a PR with a fix, that'd be great!

[rmosolgo]

I blew the dust off these tests and got them passing, but I'm not sure it's the right implementation.

After my changes, a Lookahead has a map of { type => ast_nodes, ... }, to handle things like name in the example above. As a result, there's not just one field for a lookahead, but n fields, since, in the example of name above, there are two different name fields involved. But for compatibility, it still pretends to be a single-typed object, with field and owner_type methods. However, these methods misbehave for multi-typed Lookaheads. I could:

• Leave it the way it is, and it would silently fail for code that encounters a multi-type situation;
• Remove the single-type methods (#field, #owner_type) and force callers to update to the new methods
• In between: raise when a lookahead has multiple types but a single-type method is called. In that case, the schema's code should be updated to use plural methods instead.

But they all seem complicated 😖

[jeromedalbert]

I'm also coming from https://github.com/DmitryTsepelev/graphql-ruby-fragment_cache#limitations. I am using that gem to facilitate caching, but because my schema has unions, some parts of my app can't be cached at the moment.

• Leave it the way it is, and it would silently fail for code that encounters a multi-type situation;
• Remove the single-type methods (#field, #owner_type) and force callers to update to the new methods
• In between: raise when a lookahead has multiple types but a single-type method is called. In that case, the schema's code should be updated to use plural methods instead.

I'm guessing point 2 or 3 would be better than 1, because with solution 1 the graphql-ruby-fragment_cache gem would still not work with unions.

[jeromedalbert]

@gsdean FYI I worked around the issue in my app by not using union types, and using interfaces instead. It seemed to mostly work.

Unions and interfaces in GraphQL are very similar, so migrating required minimal effort. No changes were needed for the frontend / API consumer, since the ... on syntax in queries remained the same.