nexus: Union's resolveType doesn't know about __typename

This is indeed an oversight and something I recently also realized/encountered as well when trying to build some union types using the same pattern from that talk.

Will look to get a fix in for this soon!

Here is a revised spec following discussion with @Weakky

1. When an object type is just a member of an unexposed union nothing special has to happen

2. When an object type is a member of a union and the union type is exposed directly or indirectly through a root field then the user will have access to three kinds of static feedback:

   1. A resolveType static error
   2. An isTypeOf static error
   3. A resolver (the one for the field whose return type is the union) return type static error

3. A breakdown of the three types

   1. resolveType error is on the union type definition config, a new field called resolveType. It will replace t.resolveType API. The reason we cannot use t.resolveType is that there is no way in TS to require, statically, for a function to be called by the user.

      unionType({
        // TS ERROR: Missing property `resolveType` ...
        name: 'ABC',
        definition(t) {
          t.members('A', 'B')
        },
      })
      

   2. isTypeOf error is on the object type definition config, a new field called isTypeOf. There is no t.isTypeOf API to replace. The reason we cannot use t.isTypeOf is for the same reason given for not using t.resolveType.

      objectType({
        // TS ERROR: Missing property `isTypeOf` ...
        name: 'A',
        definition(t) {
          // ...
        },
      })
      

      objectType({
        // TS ERROR: Missing `isTypeOf` ...
        name: 'B',
        definition(t) {
          // ...
        },
      })
      

   3. Resolver return type error is for every instance of a field whose type is that union.

      queryType({
        definition(t) {
          t.field('abc', {
            type: 'ABC',
            resolve() {
              // TS ERROR: Missing property `__typename: "A" | "B" | "C"` ...
              return ...
            }
          })
          t.list.field('abcs', {
            type: 'ABC',
            resolve() {
              // TS ERROR: Missing property on each object `__typename: "A" | "B" | "C"` ...
              return ...
            }
          })
        },
      })
      

4. There are problems with the static errors as a means of feedback. The three types of error feedback happen at once, solving any category of them is all that is needed, but this is not obvious. It will not even be obvious necessarily to parse the categories of errors in one’s own mind. TS will just display all the static errors in their raw unrelated and ungrouped form.

   For this reason there will be a configuration option to enable categories of feedback:

   makeSchema({
     checks: {
       unions: {
         isTypeOf?: boolean // default true
         resolveType?: boolean // default false
         backingType?: boolean // default false
       }
     }
   })
   

   By default Nexus will only give the isTypeOf static feedback. However for teams working with different standards, they may change to enable a different set of the union checks.

5. To aid the feedback, there will be rich runtime feedback that gives more in-depth explanation about what and where the issue is, and how and where to fix it.

6. jsdoc will be used extensively to help guide the developer

7. website doc will be extensive too

References

• https://github.com/graphql/graphql-js/blob/master/src/execution/execute.js#L1157-L1195
• https://github.com/graphql/graphql-js/issues/876#issuecomment-304398882
• https://github.com/Microsoft/TypeScript/pull/29478

So all unions will have to be written as:

export const PostResult = unionType({
  name: 'PostResult',
  description: 'A Post, or NotFound if the post was not found',
  definition(t) {
    t.members('Post', 'NotFound');
    t.resolveType();
  }
});

Wouldn’t it be better to do:

export const PostResult = unionType({
  name: 'PostResult',
  description: 'A Post, or NotFound if the post was not found',
  definition(t) {
    t.members('Post', 'NotFound');
  }
});

Or even:

export const PostResult = unionType({
  name: 'PostResult',
  description:
    'A Post, or NotFound if the post was not found',
  members: ['Post', 'NotFound']
});

The tricky thing with typings here is that if you are using the default execution algorithm, __typename should be required for member types, but if you have a custom one then we shouldn’t necessarily require __typename be specified, since that’s the point of defining the custom resolveType.

Thinking about this.

• Restating the problem: GraphQL Clients, tools, and fragments use __typename field on members of a returned union type to reconstruct what types are being dealt with at runtime. This is a special spec-compliant field that does not show up in the GraphQL SDL Schema. Nexus Schema allows developers to provide this data one of two ways:

  1. In the resolver for the field returning a union type all the objects being returned are “marked”/“branded”/whatever with a property __typename whose string value matches exactly the name of the GraphQL object type that the data conforms to (graphql.js raises standard error upon non-conformance).
  2. In the union type definition a special method is implemented called t.resolveType. It runs for each object being returned for that union at runtime. Each run must result in the string value that matches exactly the name of the GraphQL object type that the data conforms too.

• If a union type definition has implemented t.resolveType then the backing types of members of the union in the context of the union field resolve do not require __typename.

• If the backing data typings for any field returning that union type are such that members of that union provide a __typename then no t.resolveType is required at the union type definition level.

• If neither of the above is true then we’re left with trying to figure out which one the developer should satisfy. Nexus cannot know. The best it can do is raise errors on both sides statically, and dynamically raise a useful runtime error during development. Unfortunately, statically, there will be no easy way to explain that two static errors are mutually exclusive and only ONE must be resolved by the developer. Docs, and runtime feedback, will be our best way to support beginners. Veterans will be fine, though may find this lack of precise feedback “annoying”.

• Note: Even if a developer provides t.resolveType they may still augment the backing data types as they wish, of course. If they augment it such that they state the backing type of an object will always have __typename. In this case it just means that the developer will have access to __typename in their t.resolveType implementation.

As I suggested, yes, though I suppose I was only looking at it as an improvement on the current alternative - rather than revisiting why it is how it is…

export const PostResult = unionType({
  name: 'PostResult',
  description:
    'A Post, or NotFound if the post was not found',
  members: ['Post', 'NotFound']
});

IIRC this API wasn’t originally possible it was due to some issues w/ type completion/safety on object members not being correct, I think I left a comment about this as a reminder for a similar issue on interfaces, need to revisit if anything has changed here:

https://github.com/prisma-labs/nexus/blob/38fe229a1971f975f6ce60c8c3a6208856d72938/src/definitions/interfaceType.ts#L17-L22

It looks like the PR referenced there got merged, so maybe it is supported now? Will give it a shot and see if we can adjust/simplify the API here.

I guess we could also make the assumption that if resolveType isn’t defined, __typename would be required on any possible union members?

Doc seems good! I am glad multiple strategies are allowed.

Very nice analysis and writeup, and the proposed solution is great in my opinion 👏

Started thinking more about this, and there’s a few things to address here.

First, I believe the messaging / behavior of the current warning message is incorrect:

... or t.resolveType(() => null) if you don't want or need to implement.

You should always implement this somehow, () => null is useless for union types.

The question is really whether you want a custom resolve or whether you just want the default behavior, as @ThisIsMissEm is re-implementing by hand with the item => item.__typename.

If you are in-fact providing the __typename, then you can use the default one. As such, I was planning on changing the messaging/behavior to:

... or t.resolveType() if you want to use GraphQL's defaultTypeResolver and hide this warning.

So essentially if you call t.resolveType() with no arguments, it’ll use defaultTypeResolver with behavior detailed here.

The tricky thing with typings here is that if you are using the default execution algorithm, __typename should be required for member types, but if you have a custom one then we shouldn’t necessarily require __typename be specified, since that’s the point of defining the custom resolveType.

@ThisIsMissEm does this sound like a reasonable approach/change to the issue you describe?

I was thinking of adding the change in the messaging / ability to call t.resolveType() for the soon to be released 0.12.0 and then address the __typename as part of the refactor of type generation in the next release.