extendSchema breaks resolution of fragments on interfaces / unions

[exogen]

I know extendSchema is undocumented, but it's the only way to accomplish the schema extensions feature I recently added to my library, so I'm reporting this in case someone over there is making extendSchema legit. :)

I've got a query of which this is a part:

{
  target {
    ... on URL {
      resource
    }
  }
}

The target field is of type Entity, which is an interface, and URL implements that interface. When I run this without using extendSchema, I get the expected result:

{
  "target": {
    "resource": "https://www.discogs.com/artist/408531"
  }
}

But when the schema has been run through extendSchema, I get a seemingly impossible result:

{
  "target": {}
}

This result is supposed to be impossible with GraphQL – you can't ever get an empty object. Either it's null, or has at least one field (which could be null).

When I log what's happening in the resolvers, the target resolver is receiving and returning all the correct data. But the URL field resolvers are never run. Somehow running the schema through extendSchema causes this.

[exogen]

So far I've narrowed this down to doesFragmentConditionMatch: the type argument it receives is the URL type instance from BEFORE the schema was extended, but the typeFromAST call that gets conditionalType returns the URL type instance from AFTER the schema was extended, and they are not the same class instance.

[exogen]

The problem is that after building the types for the new schema, the .ofType references in other field types haven't been updated to the newly generated types (types seem to be rebuilt even if they are not modified by the new schema, like URL in this case: prevSchema._typeMap.URL !== newSchema._typeMap.URL, but the types in newSchema._typeMap haven't had their ofType references updated).

[exogen]

Ah, I was able to fix this in my own code, here's how. The problem ended up being my interface's resolveType definition. If you extend the schema, it's pretty much not possible to define resolveType in a reasonable way. Like in the example, it was returning the types I had originally defined, e.g.:

  resolveType(value) {
    if (value instanceof Dog) {
      return DogType;
    }
    if (value instanceof Cat) {
      return CatType;
    }
  }

But this won't be correct after the schema is extended, because DogType and CatType will have been replaced with new instances in the new schema's type map, and they won't compare equal to the old ones. So I had to do essentially this:

  resolveType(value, context, info) {
    const typeMap = info.schema.getTypeMap()
    if (value instanceof Dog) {
      return typeMap[DogType.name];
    }
    if (value instanceof Cat) {
      return typeMap[CatType.name];
    }
  }

Which is unfortunate. (Note that the current docs for resolveType are also incorrect – they say resolveType receives value and info, but it actually receives value, context, then info. I actually think it's also still possible that some ofType references in the resulting schema are stale, but I'm not positive and was able to fix it this way.)

Also leaving a note here that nodeDefinitions from Relay suffers from the same issue – unless you return the type instance from info.schema.getTypeMap(), an extended schema will break your queries.

[leebyron]

extendSchema is not mutative - it will produce new versions of all modified types leaving the existing type references (and the existing schema reference) unaltered.

That means any references to the original types remain original.

One way around this when implementing resolveType is to return types as string names instead of reference:

resolveType(value, context, info) {
    const typeMap = info.schema.getTypeMap()
    if (value instanceof Dog) {
      return 'Dog';
    }
    if (value instanceof Cat) {
      return 'Cat';
    }
  }

If resolveType returns a string, that string is expected to be a type name and it's looked up from the executing schema - which if that is your extended schema, you'll get the extended type.

[exogen]

@leebyron I didn't realize that was possible but was actually going to be my suggestion for how to improve this, that's good to know! I think there are still several actionable improvements from this issue:

1. Document that you can return strings from resolveType. The docs don't mention it and even insist that the return type is ?GraphQLObjectType.
2. Actually do that in the examples. Since you can't control whether some consumer down the line is going to extend your schema, you could unknowingly be preventing it by returning the actual GraphQLObjectType references as in the example. It's not clear that you'll run into this issue unless you've dealt with it before.
3. Why did my original query just return {} instead of throwing an error? That would have helped a lot. Again, according to my understanding of GraphQL up to that point, I thought that was an impossible query response.