mickhansen / dataloader-sequelize

Batching and simplification of Sequelize with facebook/dataloader
MIT License
284 stars 58 forks source link

feature: findOne/All where id IN (1, 2, 3) #139

Closed intellix closed 1 year ago

intellix commented 1 year ago

We're using dataloader throughout and one scenario I noticed doesn't work is the IN operator:

await Asset.findAll({
  [EXPECTED_OPTIONS_KEY]: context.dataloaderContext
  where: {
    id: {
      [Op.in]: assetIds,
    },
  },
});

Hopefully I can find some time to add it in, if it's possible. Do you have any pointers of how I might be able to achieve it or if it's possible at all?

mickhansen commented 1 year ago

@intellix findAll isn't covered at all by this library currently.

A workaround would be to do something like this:

await Promise.all(assetIds.map(assetId => {
  return Assets.findByPk(assetId, {[EXPECTED_OPTIONS_KEY]: context.dataloaderContext});
}));

Of course if one were to remove the dataloader context that would now be ineffiecient.

intellix commented 1 year ago

cheers for the workaround, we're gonna put it in for now.

If we were to contribute the merging of queries with the same findOne/findAll, do you have any tips or is it no-mans land?

mickhansen commented 1 year ago

cheers for the workaround, we're gonna put it in for now.

If we were to contribute the merging of queries with the same findOne/findAll, do you have any tips or is it no-mans land?

For certain cases of findAll we could hook in and attempt to deduce if the query is mergeable, but it's likely to be a bit of a mess.

stale[bot] commented 1 year 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.