graphql / graphql-js

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

Support asynchronous resolveType method for abstract types #398

Closed taion closed 7 years ago

taion commented 8 years ago

This might be more of a "does this make sense" question than a feature request, but does it make sense for resolveType to optionally be async on abstract types, the way that field resolution can be optionally async?

Or would this be weird given that resolveType always runs, while any particular field may not be required for a given query?

leebyron commented 8 years ago

Is there a use case you have in mind for an async resolveType? Given that it's called from within field execution, it's plausible that support for this could be added, but it would add some complication to the executor and we would definitely want to ensure there's some compelling use case.

taion commented 8 years ago

This comes up for a sort of weird reason.

In a lot of places in my GraphQL server, instead of passing around objects, I pass around "object stubs", so e.g. just something like { id: '5' } or something.

Then, in my resolve function, I use DataLoader to retrieve the object if needed. The reason I do this is that it allows me to efficiently handle nested queries that look like:

fragment on Widget {
  nestedObj1 {
    nestedObj2 {
      nestedObj3 {
        field
      }
    }
  }
}

Where these nested objects share the same local ID, but e.g. correspond to different endpoints on my backend, as it allows me to skip querying the endpoints for nestedObj1, nestedObj2, and nestedObj3 (assuming that I expect them to exist).

I want to explore using GraphQL types to control data fetching – imagine something like having ActiveTodo and CompletedTodo types, where the concrete type is determined by the value of some fields for the backend. This ought to then allow me to do stuff like fetch a list of todos, but only pull down fields as relevant for displaying active or completed todos as needed.

However, for consistency with the "stub object" thing above, I may not in general have access to the status synchronously when attempting to resolve the type.

So anyway it's an intersection of two somewhat weird use cases, and comes from wanting to maintain consistency in doing this across a codebase.

taion commented 8 years ago

To forestall the XY problem, I wrote up what I'm actually trying to accomplish in #405.

sonewman commented 8 years ago

I definitely have a use case for this, and was going to create an issue if the wasn't one.

I work for one of the largest grocery retailers in Europe and am currently leading development of a new gateway API for our online grocery shopping.

This API is purely being driven by Graphql.

In our use case we have multiple different backend services and new ones being developed to replace them as well as different business units with separate API services.

To combat this we are heavily using interfaces to resolve different mapping types, as well as dynamically resolving resources to fetch our data.

Because of the interrelated & recursive nature of our data structures we need our types to be atomic and in control of their data fetching rather than reliant on the parent node to provide this.

Our API is in essence calling many different API's rather than formulating database queries.

By having resolveType asynchronous it would allow us to preload data required for that type, which would then make our field resolution much simpler.

We do not use the 'isTypeOf' API, but I guess it could be inconsistent if one was async and the other was not?

sonewman commented 8 years ago

I'm happy to take a stab at a PR for this if you think its ok conceptually and doesn't violate the specification.

The alternative is that we write an alternative standalone executor to provide this functionality.

sonewman commented 8 years ago

I've been thinking about this, I think a nicer solution could be one of two options.

Either an extra optional ObjectType Config attribute method (that can be async), which would be resolved before starting to resolve object fields, the return value of which would become the first argument of objects field resolvers.

The alternative would be the ability to create "higher order types" that can essentially intercept the resolution flow to provide a similar functionality. The former option would be the simplest to implement, while the latter could provide scope for even more interesting use-cases. For instance it could even be possible to intercept type resolution before and after, similar to the visit() API, except supporting asynchronous behaviour. I guess it could be some kind of ProxyType...

taion commented 8 years ago

@sonewman

BTW, I ended up not needing this, and went with https://github.com/graphql/graphql-js/issues/405#issuecomment-234085728 instead. I'm not sure if that's applicable to you, though.

In your case, though, unless the GraphQL fields are polymorphic, I'd consider moving that resolution out of the GraphQL type itself, and instead have the GraphQL type wrap some sort of instance of a proper model type.

sonewman commented 8 years ago

@taion Yeah, i don't think that really solves the problem. The main issue is that we want to make sure that we do no over fetch, so we only make calls for the data we need.

The problem then is that we have calls using dataLoader for each separate node.

The problem then is that we need to make subsequent calls off the back of the first calls. But those calls need to be batched using dataLoader also. At the moment we have three fields making three separate calls, which should be aggregated into one because they are asynchronous, we have no way to guarantee the are called in sequence.

I know our use case is probably hard to visualise, but i guess some of our requirements are pushing the boundaries of the graphql resolving pattern.

In some cases it is advantageous for us to be able to view the selectionSet below a given node up front and make strategic decisions about what calls to make before it comes to resolving a given leaf node.

I wanted to check if there was a way i could push this kind of idea/notion in graphql before developing something custom to fit our use case.

cagdastulek commented 7 years ago

I am kind of new to graphql and I may be doing a newbie mistake or pushing the use case for Unions too much but one thing I liked GraphQL is to return a different type of object field by calculating the type in __resolveType. This way I can do such things:

type Product {
  details: ProductDetails
  ...
}
union ProductDetails = PublicDetails | PurchasedDetails

type PublicDetails {
  foo
  bar
}

type PurchasedDetails {
  foo
  bar
  onlyForPaidCustomers
}

And say, in resolveType of ProductDetails, it would be great:

If there is any better way of doing this without relying on resolveType, I would be happy to hear. But from my general experience, so far, it feels like resolveType would be a very cool place to solve this issue, but for that __resolveType should support async functions, returning Promises.

dherault commented 7 years ago

+1 for this feature, also for allowing isTypeOf to return a Promise. My use case is the following: The 'source' I pass around is a reference to remote data (an id/IRI string). So when I call resolveType or isTypeOf I need to query a database to know the result.

I can PR but I'd like to know if the maintainers are willing to see this feature implemented.

taion commented 7 years ago

This was fixed a while ago in #631.