Combine `subscribe` with `execute`?

[ericls]

Right now, subscribe and execute are two functions. And there are some issues with that.

1. Semantically, subscription is a type of execution. In the spec, Query, Mutation and Subscription are on the same level. Subscription is not an outlier.
2. When I get a Document, I need to go through validations to get the Operation, and then call execute or subscribe based on Operation's type, and they go through a similar validation, resulting in duplicated work.

[IvanGoncharov]

@ericls Main difference is in the return value since subscribe returns promise to async iterator: https://github.com/graphql/graphql-js/blob/0d2220f0a64238473f4e1c9aa8f73f891d0fd3e0/src/subscription/subscribe.js#L77

Another big difference is that execute can return the result without wrapping it in a promise: https://github.com/graphql/graphql-js/blob/0d2220f0a64238473f4e1c9aa8f73f891d0fd3e0/src/execution/execute.js#L157

and then call execute or subscribe based on Operation's type, and they go through a similar validation,

Both execute and subscribe doesn't do the validation.

resulting in duplicated work.

Since both functions accept args as objects you can share most of the code:

const args = { schema, document, /* ... */ };
switch(getOperationRootType(document, operationName).operation) {
  case 'query':
  case 'mutation':
    return execute(args);
  case 'subscribe':
    const asyncIter = await subscribe(args);
    // send event's using asyncIter
}

[ericls]

@IvanGoncharov, Thank you for you reply!

But both execute and subscribe calls buildExecutionContext, which validates and selects an operation from Document. And the signature of getOperationRootType is (s: Schema, o: Operation) which requires operation to be known.

[IvanGoncharov]

And the signature of getOperationRootType is (s: Schema, o: Operation) which requires operation to be known.

@ericls Sorry for the confusion I confused it with: https://github.com/graphql/graphql-js/blob/0d2220f0a64238473f4e1c9aa8f73f891d0fd3e0/src/utilities/getOperationAST.js#L14-L17

But both execute and subscribe calls buildExecutionContext, which validates and selects an operation from Document.

It's pretty basic validation (I thought you mean full query validation made by validate) and as you correctly pointed out code is shared code is already extracted into buildExecutionContext.

---

I agree that in theory, it would be great to unify the execution of query, mutation and subscription under a single function. In practice, it would be a breaking change and result in function that returns ExecutionResult | Promise<AsyncIterator<ExecutionResult> | ExecutionResult> which is not the best DX.

So it's something that we case explore post 15.x.x and see if someone can come up good API for such function and a plan on how to minimize breaking change.

[ericls]

@IvanGoncharov , thanks again for you reply.

buildExecutionContext is indeed very helpful. I think I'll use it for now.

[jasonkuhrt]

Just happened this use-case. As a user, my initial expectation was that I would be able to do query.execute. I was initially surprised to learn that subscription was a separate method.

But I sympathize with how the polymorphic type this would result in could hurt DX.

A new TS generic parameter and/or casting could do their bit to help, but only modestly.

Most of the time, in practice, what would probably happen, is users narrowing the result before using it.

In the end, having the API allow to do this branching up front (method selection vs result narrowing) seems very reasonable.

I don't see a "best" option here. Maybe both, e.g.:

graphql.execute(...) // ExecutionResult | Promise<AsyncIterator<ExecutionResult>
graphql.executeSubscription() // Promise<AsyncIterator<ExecutionResult>
graphql.executeQuery() // ExecutionResult
graphql.executeMutation() // ExecutionResult

I don't know if splitting Query / Mutation for API symmetry has any functional benefit here. Maybe that's acceptable, not sure.

[jamiter]

With the introduction of @defer and @stream the execute function actually can return an async iterator. This might be a great time to also allow subscription execution via the execute function.