graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.06k stars 2.02k forks source link

Support async generators as query resolvers for Array types #3445

Closed Cito closed 2 weeks ago

Cito commented 2 years ago

Currently, query resolvers for Array types can return not only JavaScript arrays, but any iterable. However, async iterables are not (yet) supported. Since GraphQL.js supports async resolvers and iterables, it would be only consequential to support async iterables here as well.

This question has been raised in #1537 and #2262 already. In the first issue, it was assumed that this would help to create a streaming interface, and I therefore dismissed and closed. But we may want to support async generators for other reasons, like those discussed in the second issue. Also, some backend services may already provide async iterable that could then be directly used in resolvers. In the first issue, I also argued that async generators are not guaranteed to finish, but the same is true for sync generators, so this is not really a reason to not support them. I thought it may be good to open a new separate issue to discuss this question.

Here is an example that shows how I expect this to work:

import { graphql, GraphQLSchema,
         GraphQLObjectType, GraphQLList, GraphQLString } from 'graphql';

// currently works only if you remove the "async" here:
async function* resolve() {
  for (const s of ["my", "values"]) yield s;
}

const schema = new GraphQLSchema({
  query: new GraphQLObjectType({
    name: 'Query',
    fields: {
      list: {
        type: new GraphQLList(GraphQLString),
        resolve
      },
    },
  }),
});
const source = '{ list }';
const result = await graphql({ schema, source })
console.log(result)

As it is, the code throws an GraphQLError: "Expected Iterable, but did not find one for field Query.list."

To change this, the completeListValue function would need to support the case that result is an async iterable. If that was supported already, we could probably just replace the Array.from by Array.fromAsync.

robrichard commented 2 years ago

@Cito I have a pull request implementing this feature: https://github.com/graphql/graphql-js/pull/2757

IMO it's a prerequisite to support the @stream directive. You can test it out with the npm version 16.1.0-experimental-stream-defer.6

Cito commented 2 years ago

@robrichard Nice, I somehow missed that. Maybe because I only searched the issues, not the pull requests.

mattkindy commented 2 years ago

Given that https://github.com/graphql/graphql-js/pull/2757 is now merged, what does it looks like for implementing this as a possible fix for https://github.com/graphql/graphql-js/issues/2262 ?

yaacovCR commented 2 weeks ago

Available in v17, closing :)