mickhansen / graphql-sequelize

GraphQL & Relay for MySQL & Postgres via Sequelize
MIT License
1.9k stars 172 forks source link

Relay, resolvers and automatic fromGlobalId unpacking #560

Closed intellix closed 4 years ago

intellix commented 6 years ago

So we're using Relay and globalIds throughout. Everything is GraphQL from scratch so we never expose the database numerical IDs to the frontend.

If we make a users call, the response looks like:

{
  "data": {
    "users": {
      "edges": [
        {
          "node": {
            "id": "VXNlcjox"
          }
        },
        {
          "node": {
            "id": "VXNlcjoy"
          }
        }
      ]
    }
  }
}

When you click on a user from a list and want to view his individual profile page, you have to make a query for him by ID obviously:

user(id: "VXNlcjox") {
  name
}

Even though it's not pretty, I'm down with this, but one thing I find myself repeating in my Schema is the unpacking of the ID to send to the resolver:

user: {
  type: UserType,
  args: {
    id: {
      type: new GraphQLNonNull(GraphQLID),
    },
  },
  resolve: (source, args, context, info) => {
    args.id = fromGlobalId(args.id).id;
    return resolver(User)(source, args, context, info);
  },
},

It feels like I'm doing something wrong but I don't think so. If I am, let me know :)

Maybe the resolver API could be simplified by checking the type of the argument and unpacking it automatically if it's of type GraphQLID. If not, perhaps it would be worth mentioning in the documentation as I feel dirty doing it and I'm sure everyone has to do the same.

mickhansen commented 6 years ago

I don't use resolver much for root calls so i'm having a hard time seeing if it's a good idea or not. I suppose as we do currently support args.id of GraphQLInt directly it makes sense to support GraphQLID too.

davidmicksch commented 6 years ago

I took the same approach as intellix regarding using the globalIds throughout our application rather than tracking the database ids as well. This would be a nice feature to have in the resolver.

mickhansen commented 6 years ago

We can either add an option to the resolver or have a special relayGlobalIDResolver.

davidmicksch commented 6 years ago

Here is my use case for encoding and decoding the globalID:

export default new GraphQLObjectType({
  name: borrower.name,
  description: 'entity for which the borrower has been created',
  fields: _.assign(attributeFields(borrower), {
    id: globalIdField('borrower'),
  }),
});
// ... delete mutation
export default {
  type: Borrower,
  args: {
    id: { type: GraphQLID },
  },
  resolve(source, args) {
    const { id } = fromGlobalId(args.id);
    return models.borrower.findById(id)
      .then(borrower => borrower.destroy());
  },
};

The way I have it setup, the encoding as a globalID is done once in the type definition and then the decoding is needed for each mutation to obtain the database id. From my perspective, the implementation is trivial, however, figuring it out took me the better part of a morning.

I wouldn't have an informed opinion on an implementation strategy, but just documenting it in the README.md may be fine and save others the time needed to figure out the pattern.

Thanks for the awesome tool! Really simplifies implementing a relay modern backend.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.