lucasconstantino / graphql-resolvers

:electric_plug: Resolver composition library for GraphQL.
MIT License
282 stars 16 forks source link

Only works if the resolver returns a promise #1

Closed thebigredgeek closed 7 years ago

thebigredgeek commented 7 years ago

https://github.com/lucasconstantino/graphql-resolvers/blob/master/src/combineResolvers.js#L15

lucasconstantino commented 7 years ago

Not really. The tests cover quite many scenarios, and in fact most resolvers in the test do not return promises. Basically, the reducer is initialized with an empty resolved promise, starting the promise chain.

The big "but" here is that combineResolvers always returns a promise, which might not be desired in some cases. Bot graphql-js and graphql-tools resolver specs receive promises just fine, in fact they wrap their resolver results in promises anyway, but if this helper method were to be used in other scenarios it would probably need some refactoring not to enforce promises when involved resolvers do not return promises. Meanwhile, it is much simpler and more elegant just to wrap the values - either error, undefineds, or true results.

thebigredgeek commented 7 years ago

Ah! I missed that part (the initial promise in the reduce call :( )! Yeah, makes sense.