v2 API RFC

[thebigredgeek]

List ideas here

[thebigredgeek]

#3

[scf4]

Since regular functions work as resolvers, e.g.:

const allPosts = (_, args, ctx) => { ... };

Is the only purpose of createResolver to allow you to compose resolvers from other resolvers?

If so, I propose the following API:

import { composable } from 'apollo-resolvers';

const deleteUser = (_, args, ctx) => { ... };
const editUser = (_, args, ctx) => { ... };
const followUser = (_, args, ctx) => { ... };
const unfollowUser = (_, args, ctx) => { ... };
const banUser = (_, args, ctx) => { ... };
const user = (_, args, ctx) => { ... };

const isLoggedIn = composable((_, args, { user }) => {
  if (!user) throw new Error('Please log in.')
});

const isAdmin = composable((_, args, { user }) => {
  if (!user.isAdmin) throw new Error('Unauthorized!');
});

export default {
  Query: {
    allPosts,
    user: isLoggedIn.compose(user),
  },
  Mutation: {
    followUser,
    unfollowUser,
    isAdmin.compose({
      deleteUser,
      editUser,
      banUser,
    })
  },
};

As for baseResolvers, perhaps combineResolvers could take an optional baseResolver parameter and compose it with every other resolver?

export default combineResolvers([
  postResolvers,
  userResolvers,
], { baseResolver });

This would result in far more manageable code IMO! 😃

[thebigredgeek]

I think this is a solid direction, but I also think there is value in handling errors as the "bubble up", so we should probably also support error resolvers in the same way that createResolvers allows.

[scf4]

I also think there is value in handling errors as the "bubble

How so? How does it differ from errorResolver.compose(newResolver) or using an errorResolver as a baseResolver with combineResolvers?

[thebigredgeek]

Well, regular resolvers are unidirectional. The request pipes down from source to sink. Errors are handled after the sink throws an error, and they bubble back up in the reverse order.

[thebergamo]

These resolvers act like Middlewares, I think the idea of composable resolvers is really good.

Keeping the "middleware" idea in mind, in Express you can use an error resolver at the end. But based on @thebigredgeek comment, the resolvers are resolved in the reverse order in error cases. Instead of extending a baseResolver you can just put it as a simple function at a position 0 of an array of these middlewares it will be simple, reusable and manageable.

// userResolver.js
import { composable as applyMiddleware } from 'apollo-resolvers';

import { isAuthenticated, isAuthorized } from '.,/middlewares';

const updateUser= (_root, args, context) => { /* do your stuffs here */ };

const userResolver = {
    Mutation: {
        updateUser: applyMiddleware([ isAuthorized, isAuthenticated, userResolver ]),
    }
}

export default userResolver;

And the error resolver is only required in a single point of application. This makes sense for you guys?

[ch-andrewrhyne]

@thebergamo that's an interesting approach, but how would you implement an error resolver in this case?

[ch-andrewrhyne]

@scf4 Sorry, I've been busy at my new job. Just starting to get back into focusing on GraphQL. I had to set up a bunch of NodeJS infrastructure first haha. I'm all for a more elegant API. So long as you can pass resolvers returned from createResolver into the compose method, and the compose method will handle the error resolver composition along with the fetch resolver composition, I think this is a great approach. I'd like to ship V2 by end of September. Would you guys like to help me implement this?

[ch-andrewrhyne]

#11

[ch-andrewrhyne]

For V2, I'd also really like to refactor into typescript to make the project more developer friendly as we extend the lib's functionality.

[thebigredgeek]

^ this is me, by the way haha. I have a second account for work

[thebergamo]

Hey @thebigredgeek I can help, just need to learn a little bit more about TypeScript.

Well, the idea of the error handler, is to have a simple function like express middleware, but in the case of express you register the middleware for error at the end chain of middlewares.

const errorHandler = (err, ...params) => {/* when an error occur, this function must to be called */};  

And in this case, will be really good if this handler can be declared just once.

[scf4]

@thebergamo @thebigredgeek how about Flow instead of TS?

[scf4]

Also, how would you feel about expanding the scope of this project to become more of a library (or very small and fairly unopinionated framework) for building GraphQL server apps with Apollo?

It's surprisingly difficult to find info on best practices and solving common problems with JS GraphQL APIs. You're really on your own, especially compared to what's available on the front end. There's a lot of reinventing the wheel.

The JS GraphQL community could seriously benefit from something like this.

[thebergamo]

@scf4 I don't know what the @thebigredgeek thinks about expanding this project to something more. IMHO is better keeping this module focused on this and create a new one to be the library to build GraphQL APIs.

It's really really hard to find best pratices for somethings :p

[ms440]

@thebigredgeek Nice package! Thanks!

The only reason I'm not using it right away is that in V.1 the baseResolver does not have an info argument. The same about createResolver function... :(

Why wouldn't you push V2 to npm (with, say, V1.1)? It won't break the backward compatibility, it's an additional feature. Meanwhile, V2 work will go on.

Anyway, thanks a lot.

[thebigredgeek]

I'm building a framework called Edge at Collective Health (my awesome new job) that will take away a lot of the boilerplate for building microservices with NodeJS, including with GraphQL, with a decorator-based DSL for defining GraphQL types, resolver bindings, etc. I'll write a blog post about it and stick it in the README once it's getting closer to finished

[ch-andrewrhyne]

@scf4 Not sure on Flow. I've used it before and I like it, but Typescript removes almost all of the developer tooling boilerplate (babel, etc). I am on the fence about using Flow rather than typescript. Would love more insight into your thoughts about this.

[thebigredgeek]

One change we are making is adding the info prop for resolvers. #9

[thebigredgeek]

#20

[thebigredgeek]

Typescript support is implemented!

[thebigredgeek]

If anyone wants to implement a compose method for resolvers, I'm happy to review a PR!

[mringer]

@thebigredgeek posted my first pass at compose in #31

[ak99372]

@thebigredgeek Has #11 been implemented?

The current version of [combineResolvers] has a different signature and I can't seem to figure out how to use it.

It would be nice if it allowed to add resolver(s) to another or a group of existing resolvers (that already have a base)

combine(r1, r2)with(r3, r4)

[mringer]

@ak99372 take a look at #31

[mringer]

@thebigredgeek I've been getting Circle CI notifications about v1 API depreciation. I'm planning to refactor the circleci.yml to conform to the v2 API sometime this week, time permitting. In any case we have till the end of the month before they drop support for v1.

[thebigredgeek]

Just saw your PR! Nice job

[thebigredgeek]

For those interested, I'm modernizing this package here: #67