Spectrum: Why spectrum isn't using data loader in the connections?

[zefexdeveloper]

I'm studying GraphQL and I found spectrum really nice to learn as it has a rich code base but I was wondering, why the connections let's say ThreadConnection (and all the others) isn't using data loader? It means that we have a N + 1 problem in the connections? What about a loader like threadByChannelIdLoader?

[brianlovin]

This is a good question, and the easy answer is: we have some lingering debt of resolvers that haven't been migrated to fully utilize dataloaders. Although there are probably some where it's not even worth the effort for certain things, but certainly we have some backfilling work to be completed still :)

[zefexdeveloper]

@brianlovin Creating the dataloader for the connection isn't hard cause you can create a loader as I mentioned, the thing is the pagination/filtering/ordering, how the pagination would look like when using the loader.

---

Answering my own question: as @leebyron said in https://github.com/facebook/dataloader/issues/71#issuecomment-276274081:

DataLoader is typically not responsible for pagination. The batching behavior it provides applies to loading many keys in one dispatch to a key-value store. It shouldn't matter if those keys are related as part of a "page" or not. Since fetching ranges of lists doesn't really fit the "load by key" model, it typically happens orthogonally to DataLoader.

So it means that we could just continue doing the connections with pagination as we are doing and after that use prime in our dataloader for future queries?!

I really wanted to hear from other developers that are involved @withspectrum to know what they think about that.

[mxstbr]

why the connections let's say ThreadConnection (and all the others) isn't using data loader? It means that we have a N + 1 problem in the connections? What about a loader like threadByChannelIdLoader?

Using dataloader for connections doesn't do anything. Think about our threadConnection:

{
  user(id: "user-123") {
    threadConnection(first: 20, after: "thread-1") {
      edges {
        node {
          ...threadInfo
        }
      }
    }
  }
}

It _should_ never happen that we load the same thread twice in this connection, as it's a chronological list of unique threads, so using dataloader doesn't do anything for connections. It doesn't save us any loading.

[zefexdeveloper]

I get what you mean, in this case here:

{
  author(id: 1) {
    books {
      id
      chapters {
        id
      }
    }
  }
}

If there's 500 books, it will do something like 502 queries to the database, one for user, one for books, and 500 for chapters.

Isn't this the same that happens on something like:

{
  user(id: "user-123") {
    threadConnection(first: 20, after: "thread-1") {
      edges {
        node {
          messageConnection(first: 20) {
            # ...
          }
        }
      }
    }
  }
}

[mxstbr]

Yeah, we don't allow queries like that. We use cost analysis to make sure incoming queries aren't too expensive or nest too deeply, see https://blog.apollographql.com/securing-your-graphql-api-from-malicious-queries-16130a324a6b?gi=c8b7ebd57f1