dataloader icon indicating copy to clipboard operation
dataloader copied to clipboard

[QUESTION] data loader batch load fails when non-data loader function is included among data-loader functions.

Open hyunhoRIDI opened this issue 4 years ago • 8 comments

I am experiencing a weird edge case with data loader.

Suppose we have functions like below.

const firstQueryBatch = (ids: string[]) => do something batch with Ids...
const secondQueryNonBatch = (id: string) => ... do something with single id
const thirdQueryBatch = (ids: string[]) => ... do something batch with ids

const callThreeQueriesWithId = async (id: string) => {

  const firstResult = await (() => {
    new DataLoader(firstQueryBatch, {
        cache: false,
    });

    return (id: string) => loader.load(id)
  })();

  const secondResult = await secondQueryNonBatch(id); // non-data loader applied function

  const thirdResult = await (() => {
    new DataLoader(thirdQueryBatch, {
        cache: false,
    });

    return (id: string) => loader.load(id)
  })();

  return {
    firstResult,
    secondResult,
    thirdResult,
  };
};

const Ids = ['1', '2', '3'];
const finalResults = await Promise.all(Ids.map((id) => await callThreeQueriesWithId(id))));

Let's say there are total three async functions where the first and the last function use dataloader, but second function is just a plain non-dataloader function.

I simply expected that 'firstQueryBatch' and 'thirdQueryBatch' would be called once with batch Ids[](as dataloader should combine parameters) and only calling 'secondQueryNonBatch' three times as it is not a dataloader function. However, when I debug the output, once it calls 'firstQueryBatch' once successfully, it ended up calling 'secondQueryNonBatch' AND 'thirdQueryBatch' three times. Dataloader fails to combine parameters for 'thirdQueryBatch' and calling it three times with parameters with single item in each array. ['1'], ['2'], then ['3'].

Could anyone explain why this is happening?

hyunhoRIDI avatar Nov 07 '21 15:11 hyunhoRIDI

Same question. It fails to batch when there're other async functions around, more specifically, when using fetch from node-fetch in the async function.

For example:

const DataLoader = require('dataloader');
const fetch = require('node-fetch');

const myDataLoader = new DataLoader((keys) => {
  console.log('keys: ', keys);
  return Promise.resolve(keys);
});
const myAsyncFunc = () => fetch('https://www.google.com');

const myFunc = async (id) => {
  await myAsyncFunc(); // dataloader doesn't batch when this function is around
  await myDataLoader.load(id);
};

myFunc(1);
myFunc(2);
myFunc(3);

With await myAsyncFunc(), the result is:

keys:  [ 1 ]
keys:  [ 3 ]
keys:  [ 2 ]

Without await myAsyncFunc(), the result is:

keys:  [ 1, 2, 3 ]

lixurea avatar Apr 22 '22 06:04 lixurea

@lixurea @hyunhoRIDI This is expected because dataloader only batches requests made in the same event loop tick. https://github.com/graphql/dataloader/issues/150

vivek-ng avatar Apr 23 '22 18:04 vivek-ng

@lixurea @hyunhoRIDI This is expected because dataloader only batches requests made in the same event loop tick. #150

werid thing is, if I put non-dataloader function behind, dataloader successfully collects the requests within a tick. Using @lixurea's example,

const myFunc = async (id) => {
  await myAsyncFunc(); // dataloader doesn't batch when this function is around
  await myDataLoader.load(id);
};

this does not work. but

const myFunc = async (id) => {
  await myDataLoader.load(id);
  await myAsyncFunc(); // this way dataloader collects within a tick.
};

could you confirm this is right @lixurea ?

hyunhoRIDI avatar Apr 24 '22 06:04 hyunhoRIDI

@hyunhoRIDI yes, if the data loader function goes first, it successfully batches requests. Not sure why though.

@vivek-ng Thanks for the info. I've been reading about event loops, but still not sure what makes it a separate event loop tick?

lixurea avatar Apr 25 '22 12:04 lixurea

@vivek-ng Is there any way to use permissions or authorization guards like directives or GraphQL Shield alongside dataloaders without .load getting hit multiple times? Was anyone else here able to solve this so far?

forrestwilkins avatar Sep 02 '23 03:09 forrestwilkins

I also asked about this issue here: https://www.reddit.com/r/graphql/comments/1686dmf/dataloaders_graphql_shield_with_apollo_server_and/

forrestwilkins avatar Sep 02 '23 16:09 forrestwilkins