duplicate variables when query contains overlapping fragments on links

[adamkl]

Intended outcome

We're using schema-stitching to create an API for use by a new UI application based on react/relay. I expect that the combined query and fragments from relay sent to the API should be parsed and delegated to the individual services that make up the stitched API, and that selection sets targeting linked fields should be handled.

Actual outcome

In certain cases, the stitched API returns null for the linked fields and an error similar to the following:

{
  "data": {
    "propertyById": {
      "bookings": [null],
      "id": "p2",
      "name": "Another great hotel"
    }
  },
  "errors": [
    {
      "locations": [
        {
          "column": 15,
          "line": 14
        },
        {
          "column": 15,
          "line": 24
        }
      ],
      "message":
        "There can be only one variable named \"_propertyId\".,There can be only one variable named \"_limit\".",
      "path": ["propertyById", "bookings"]
    }
  ]
}

How to reproduce the issue

I've found that certain fragments that overlap when selecting a linked field (i.e. two different fragments both specify a linked field in their selection set) results in this issue. I believe the likely cause is this:
https://github.com/apollographql/graphql-tools/blob/e9c0eb56c41dffc5e04644c2650cd0080ed77aa4/src/stitching/delegateToSchema.ts#L217
And I've created a fork with some extra tests that recreate the scenario:
adamkl@743894b

Next steps

I'm going to start working on a fix for this, because its a blocker for us right now. If you guys have given any thought to this (based on the comments in the code, it looks like you might have suspected it might be an issue), let me know, and I'll adopt whatever approach you suggest, otherwise I'll see what I can come up with. Look for a PR.

[adamkl]

Update

I think I have a workable solution to generating unique variable names, but this has now resulted a different error:

{
  "data": {
    "propertyById": {
      "bookings": [null],
      "id": "p2",
      "name": "Another great hotel"
    }
  },
  "errors": [
    {
      "locations": [
        {
          "column": 15,
          "line": 14
        },
        {
          "column": 15,
          "line": 24
        }
      ],
      "message":
        "Fields \"bookingsByPropertyId\" conflict because they have differing arguments. Use different aliases on the fields to fetch both if this was intentional.",
      "path": ["propertyById", "bookings"]
    }
  ]
}

This makes sense since under the hood the bookings property is being delegated to bookingsByPropertyId, and since I'm now using unique variable names, my query is trying to select bookingsByPropertyId twice with different arguments, generating the error.

Next steps

I'm thinking I'll have to find a way to alias these with unique ids similar to what is being done with the variables. I'll admit that I'm getting out of my depth at this point, so any suggestions would be appreciated.

[goldensunliu]

I think this bug is related to the TODO here https://github.com/apollographql/graphql-tools/blob/master/src/stitching/delegateToSchema.ts#L216

[goldensunliu]

I am also trying to create aliases on all the fragment definitions however, it still outputs the same error since I assume the code is generating variables based on the actual field, not the aliases

[goldensunliu]

@adamkl I haven't found a way around it. right now I just have to take the hit and assume the client is asking the right fields for the resolver to work correctly. It is def not the graphql way of course...

[adamkl]

Turns out that I was barking up the wrong tree when it came to generating unique variable ids (at least for my issue). All that was required was a simple deduplication of the generated variables to get this to work. See #607.

[stubailo]

Looks like the fix was merged!