Relay creates invalid query with union type where one type implements Node

[calebmer]

Relay is generating an id field on a union type where all types in the union type do not implement Node. Here’s a schema (with names obfuscated):

type A implements Node {
  b: B!
}

# Not actually sure if this is the correct syntax, don’t think i’ve ever seen it :blush:
union B = C | D | E

type C {
  c: String!
}

type D implements Node {
  id: ID!
  d: String!
}

type E {
  e: String!
}

With a query that looks like:

{
  a {
    b {
      type: __typename
      ... on C {
        c
      }
      ... on D {
        d
      }
      ... on E {
        e
      }
    }
  }
}

Relay generates an id field so the query that gets sent looks more like:

{
  a {
    b {
      type: __typename
      # id field generated by Relay
      id
      ... on C {
        c
      }
      ... on D {
        d
      }
      ... on E {
        e
      }
    }
  }
}

This of course fails on the server.

Edit: In the original post, I omit that A implemented Node. A implements Node was therefore retrospectively added. See this comment.

[yuzhi]

cc @josephsavona

[josephsavona]

Weird. We should only be generating an id field when the parent type implements Node; otherwise we add a ...on Node {id} instead (which would work here). That logic is in RelayQLPrinter. I'll try to think of a short-term workaround. In the meantime, you might try logging in the babel plugin and try to figure out what's going on (why is hasField('id') true for that union?)

[calebmer]

@josephsavona Turns out in this instance A does implement Node. Didn’t know that was important. I’ve updated the schema.

That still shouldn’t be correct though right?

[NevilleS]

The fact that A implements node isn't what Joe means by the "parent type" here - the parent of the faulty id field you're describing is the union type B. B shouldn't have an id field, but the RelayQLPrinter is getting fooled into thinking it does.

[NevilleS]

You should check your schema.json for the definition of the B type. It should look something like this:

{
  "kind": "UNION",
  "name": "B",
  "description": null,
  "fields": null,
  "inputFields": null,
  "interfaces": null,
  "enumValues": null,
  "possibleTypes": [
    {
      "kind": "OBJECT",
      "name": "C",
      "ofType": null
    },
    {
      "kind": "OBJECT",
      "name": "D",
      "ofType": null
    },
    {
      "kind": "OBJECT",
      "name": "E",
      "ofType": null
    }
  ]
}

I suspect your union type B actually has fields, so your schema isn't quite what you are sharing here...

[josephsavona]

What @NevilleS said :-)

[calebmer]

Here’s what the introspection query result is. Names and descriptions are obfuscated of course.

{
  "kind": "UNION",
  "name": "B",
  "description": null,
  "fields": null,
  "inputFields": null,
  "interfaces": null,
  "enumValues": null,
  "possibleTypes": [
    {
      "kind": "OBJECT",
      "name": "C",
      "ofType": null
    },
    {
      "kind": "OBJECT",
      "name": "D",
      "ofType": null
    },
    {
      "kind": "OBJECT",
      "name": "E",
      "ofType": null
    }
  ]
}

[NevilleS]

Hm. Alright, I'd recommend you add a snippet like this to RelayQLPrinter.printField:

      if (fieldType.getName({ modifiers: false }) == "B") {
        console.log("checking B type...")
        console.log("hasField('id')", fieldType.hasField(FIELDS.id))
        console.log("shouldGenerateIdFragment", shouldGenerateIdFragment(field, fieldType))
      }

[kassens]

Closing this as it refers to classic where we don't expect to do any major updates, please re-open if this still applies to the modern compiler.