sikanhe / gqtx

Code-first Typescript GraphQL Server without codegen or metaprogramming
458 stars 13 forks source link

Add required resolve function to subscriptions #67

Closed mbirkegaard closed 9 months ago

mbirkegaard commented 2 years ago

This adds a resolve function to subscription fields.

As per the spec, graphql-js calls resolve on each event in the stream returned by subscribe. However, when using the type factory in gqtx the resolve function is just the identity.

In other words, if subscribe yields values of type Event that are not compatible with the field type (Out), we need to be able to supply resolve to map from Event to Out.

A neater and non-breaking solution would be to only require the resolve function if the type of the event in the stream is not compatible with the output type of the field, but I was unable to get that working using an Event extends Out ? ... trick similar to how the resolve function is optional for fields if Src[TKey] extends Out.

An attempt at making it optional is here https://github.com/mbirkegaard/gqtx/commit/c1355124fe72d5ef516659cf9027da31d22e071e, but various attempts failed with the compiler error

'Out' could be instantiated with an arbitrary type which could be unrelated to 'Event'

despite Event extends Out

mbirkegaard commented 2 years ago

@mbirkegaard Hey, can you please rebase your branch onto master, so the CI checks run on this PR?

Done

mbirkegaard commented 2 years ago

Casting like this in define.ts

resolve: opts.resolve ?? ((v: Event) => v as unknown as Out)

makes the argument to resolve unknown which makes this typecheck

  // subscribe yields an Event type that does not extend Out
  // so this does not type check
  t.subscriptionField({
    name: 'foo',
    type: t.Boolean,
    subscribe: async function* () {
      yield { bar: true };
    },
    resolve: (p) => p,
  });

instead of erroring with

type { bar: boolean } is not assignable to bool
mbirkegaard commented 2 years ago

I updated the references to the spec and graphql-js in the description, but I'm a bit confused about whether the behavior I've read into this is actually what the spec says it should be.