search

juancastillo0 / leto

Dart GraphQL server libraries. Utilities, code generator, examples and reference implementation.
https://juancastillo0.github.io/leto/
MIT License
41 stars 6 forks source link

Dataloader: Wrong assumption about the batch size and response size #27

Closed luissantos closed 3 months ago

luissantos commented 4 months ago

https://github.com/juancastillo0/leto/blob/8778144bce1ba0dbf65253ee1d78316d2162cbc1/leto/lib/dataloader.dart#L308

It is possible that the loader gets multiple requests for the same entity in the same batch. Naturally the downstream repository/service will perform a de-duplication and return only one instance.

Is also possible for entity to not be found, In this case the entity should have been resolved to null.

In both situations the load throws an error when it shouldn't.

juancastillo0 commented 3 months ago

Hi thanks for the issue!

The implementation is based on https://github.com/graphql/dataloader, I think you have to return null if the value is not found. But I may be mistaken, perhaps their documentation has more information. I think we should keep the original behavior, however if you believe the implementations are different then we can merge a PR.

Thanks again!

luissantos commented 3 months ago

@juancastillo0 After opening the issue I looked through the code an realized that the mapping is done based on order and not based on the keys. An implementation based on a map would also had its problems so I now believe that there is much advantage to a different solution.