mickhansen / graphql-sequelize

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

Question: How to use it with dataloader-sequelize #656

Closed bullsei closed 5 years ago

bullsei commented 5 years ago

Heey,

I am kinda new to this stuff. In the docs it says "Should be used with dataloader-sequelize to avoid N+1 queries" and now i am wondering whats the best way to combine this 2 packages. I am not sure how i need to pass the context created by createContext() into the resolver (resolver from graphql-sequelize). or how i need to pass the context into my sequelize methods. Is this even necessary or is it already working magically since i pass my sequelize connection into createContext() and my models into resolver? i found this issue that probably could have answered this question but all the code is gone now #645 . Thanks in advance :)

bullsei commented 5 years ago

I digged a bit more into this stuff and i found out that it was apparently already working. I tried some stuff out with the graphql-yoga in examples folder and i noticed that all it needs to get batching working is to use createContext(). So batching is working fine with the server.js file like this

createContext(models.sequelize); const server = new GraphQLServer({ typeDefs, resolvers, context(req) { return {}; } });

As you can see i i don't pass anything into the context and the batching is still working fine for me. Now i wonder why it is even necessary to pass the dataloaderContext into the context when it works fine just without it, what am i missing here?

mickhansen commented 5 years ago

Technically by calling createContext all the methods are stubbed out, but you only get batching, not per request caching.

bullsei commented 5 years ago

Thanks for the reply. So i went back to passing it into context. I will play around more with this :)

intellix commented 5 years ago

I'm thinking that the docs/references are out-of-date with the latest dataloader-sequelize 2.0 released a week ago and I can update them if I'm on the right track.

My original usage stopped working after updating:

// Globally enable dataloader on server start
const dataloaderContext = createContext(sequelize);
resolver.contextToOptions = { [EXPECTED_OPTIONS_KEY]: dataloaderContext };

After a bit of playing I think I've got it:

const { resolver } = require('graphql-sequelize');
const { createContext, EXPECTED_OPTIONS_KEY } = require('dataloader-sequelize');

...

const apolloServer = new ApolloServer({
  schema,
  context: ({ req }: { req: express.Request }) => {

    const dataloaderContext = createContext(sequelize);

    // Magic swap for key in context
    resolver.contextToOptions = {
      dataloaderContext: [EXPECTED_OPTIONS_KEY],
    };

    return {
      request: req,
      dataloaderContext,
    };
  },
});

Then your resolvers need to be lazily instantiated on a request basis so:

Message: {
  id: globalIdField(Message.name),
  user: resolver(() => Message.associations.user),
},

Becomes:

Message: {
  id: globalIdField(Message.name),
  user: (...args) => resolver(() => Message.associations.user)(...args),
},

This part was the weird stuff that required a lot of debugging:

resolver.contextToOptions = {
  dataloaderContext: [EXPECTED_OPTIONS_KEY],
};

Honestly think I was lucky to stumble upon that and get it working tonight but I still wonder if I'm using it as intended cause it doesn't feel right. Am I on the right track @mickhansen?

mickhansen commented 5 years ago

Hmm you're right @intellix there seems to be an inconsistency there.

contextToOptions adds stuff from context to options when calls go through graphql-sequelize. So the target option key should in fact be EXPECTED_OPTIONS_KEY since that is what dataloader-sequelize wants. Looks like i might have messed up the documentation.

resolvers should work as is though?

intellix commented 5 years ago

Resolver worked fine

mickhansen commented 5 years ago
resolver.contextToOptions = {
  dataloaderContext: [EXPECTED_OPTIONS_KEY],
};

Is definitely the correct way. Assuming you set context.dataloaderContext. In my own projects i just set context[EXPECTED_OPTIONS_KEY]

caioflores commented 5 years ago

After many attempts I made it work with

schema.context = req => {
  const dataloaderContext = createContext(sequelize);

  resolver.contextToOptions = {
    dataloaderContext: [EXPECTED_OPTIONS_KEY]
  };
  return {
    ...req,
    dataloaderContext
  };
};

We should update example and README with this.

caioflores commented 5 years ago

Created a issue to solve this #670