TriPSs / nestjs-query

Easy CRUD for GraphQL.
https://tripss.github.io/nestjs-query/
MIT License
164 stars 46 forks source link

Custom Assembler: TypeORM module inconsistently calling async methods #215

Closed jamesmcglinn closed 10 months ago

jamesmcglinn commented 10 months ago

Describe the bug I'm adding computed fields to a DTO, and those fields sometimes require calling async methods from custom services.

To do this I've created a custom Assembler which overrides convertAsyncToDTO() and convertAsyncToDTOs(), similar to the method described here: https://tripss.github.io/nestjs-query/docs/concepts/advanced/assemblers#classtransformerassembler

This works well when querying for this DTO directly, but when querying as a relation these methods aren't called. Instead the non-async methods convertToDTO() and convertToDTOs() are called. Unfortunately as I need to call async methods to return the computed fields this doesn't work.

Have you read the Contributing Guidelines?

Yes

To Reproduce Steps to reproduce the behavior:

  1. Create a custom Assembler, overriding convertAsyncToDTO() and convertAsyncToDTOs().
  2. Query for these items as a relation of another item.

Expected behavior The custom Assembler async methods should be called to convert the entities to DTOs.

Desktop (please complete the following information):

Additional context This appears to be caused by the TypeORM RelationQueryService inconsistently calling the assembler's async methods.

For example queryRelations() calls assembler.convertAsyncToDTOs(), but findRelation() calls assembler.convertToDTO().

TriPSs commented 10 months ago

Could you add this case to the examples? Tests will fail and I can than take a look at it.

TriPSs commented 10 months ago

After digging a bit in this, it is not really an inconsistency because the assembler async methods are meant for when the provided data is a promise.

However I do agree that it's better to only use the async methods in the lib so users can use it more freely, maybe in the next major version we can rename this to better reflect what it will do.

TriPSs commented 10 months ago

Maybe it's even better to just add support for async to all the convert* methods.

jamesmcglinn commented 10 months ago

Completely agree @TriPSs async support for the convert* methods would make it much easier to extend assemblers.

TriPSs commented 10 months ago

@jamesmcglinn created a PR for this here #216

jamesmcglinn commented 10 months ago

Thank you @TriPSs!