graphql / dataloader

DataLoader is a generic utility to be used as part of your application's data fetching layer to provide a consistent API over various backends and reduce requests to those backends via batching and caching.
MIT License
12.89k stars 515 forks source link

[Feature request] Remove same length check or auto full fill #233

Open ash0080 opened 4 years ago

ash0080 commented 4 years ago

What problem are you trying to solve?

“DataLoader must be constructed with function which accepts Array and returns Promise>, but the function did not return a Promise of an Array of the same length as the Array of keys”

Describe the solution you'd like

In practice using a NoSql database like mongodb, some referenced document missing is not a serious problem, but this length check make things terrible, for example

user1 = { id: ObjectId(user1) }   
user2 = {id: ObjectId(user2) }
// user3 has been deleted, but associated data has not been cleaned yet.

post1 = {id: ObjectId(post1), author: ObjectId(user3) }  // here reference user3 is not exist now
post2= {id: ObjectId(post2), author: ObjectId(user1) }
post3= {id: ObjectId(post3), author: ObjectId(user2) }

// now if I make a dataloader for Users,  the array length obviously not equal !!
const usersLoader = new DataLoader(
keys => async (keys, {Users}) => {
  return await Users.find({id: {$in: keys}}).toArray()
}, 
{ cacheKeyFn: key => key.toString() }
)

Describe alternatives you've considered

My questions are:

  1. Is this length check necessary?or is a little bit over coupled feature?
  2. what's the correct pattern to deal with document missing with dataloader?
tuananh commented 4 years ago

A simple map function would do, like in the example https://github.com/graphql/dataloader/blob/master/examples/Redis.md

resolve(results.map((result, index) =>
      result !== null ? result : new Error(`No key: ${keys[index]}`)
    ));
ash0080 commented 4 years ago

Thanks, I'm using a custom mapping function to deals with the problem temporarily, but still want to discuss this issue.

besides, the behaviors from redis mget and mongodb $in are totally different.

mget return a same length array with nil inside, Actually this does not fit the situation discussed in this post.

but with mongodb, no matter using $in or mapreducer, can not ensure a equal length return

tuananh commented 4 years ago

Thanks, I'm using a custom mapping function to deals with the problem now, but still want to discuss this issue.

besides, the behaviors from redis mget and mongodb $in are totally different.

mget return a same length array with nil inside, Actually this does not fit the situation discussed in this post.

but with mongodb, no matter using $in or mapreducer, can not ensure a equal length return

agree, redis is a bad example because it return null for non existent key. however the sql example is still applied here

https://github.com/graphql/dataloader/blob/master/examples/SQL.md

ash0080 commented 4 years ago

Yes, the sql example is similar what i did now, but what I doubt is the necessity of length inspection, In GQL, if some field is missing , at least the GQLError will show the path and location of what happen, but now, it only shows an alert without any useful information.

that's why I think this feature is over coupled

LinusU commented 4 years ago

@ash0080 if you do not return an array of the same length, how will Dataloader know which input corresponds with which output?

e.g. if Dataloader gives you the array ['alice', 'bob', 'charlie'], and you return [User { Charlie }, User { Alice }], how does Dataloader know what to return?