nartc / mapper

🔥 An Object-Object AutoMapper for TypeScript 🔥
https://automapperts.netlify.app/
MIT License
959 stars 84 forks source link

mapAsync : Instructions executed in the wrong order #333

Closed llecorguille closed 2 years ago

llecorguille commented 2 years ago

Describe the bug Hello everyone, I am struggling with the mapAsync function. I am trying to map a sync source to a async destination The mapAsync function is not waiting the afterMap function to complete before resolving the promise.

Models/DTOs

I want to map a "IdOnlyDto" class to a "Dalle" class. The Dalle class is a sequelize model.

export class IdOnlyDto{
    @AutoMap()
    @IsNumber()
    id!: number;

    constructor(...args: any[]){
        this.id = args[0];
    }
}
@Table
export class Dalle extends BaseModel<Dalle> {
     @AutoMap()
    id!: number

    @AutoMap()
    idBatiment!: number

    @AutoMap()
    idSalle?: string

    @AutoMap()
    idBande?: number

    @AutoMap()
    cote!: number

    @AutoMap()
    distance!: number

    @AutoMap()
    longueur!: number
}

Mapping Configuration

mapper.createMap(IdOnlyDto, Dalle)
        .afterMap(async (source, destination) => {
            console.log("2")
            // dalleService.getById(id) returns an Observable<Dalle>
            const res = await firstValueFrom(dalleService.getById(source.id))
            Object.assign(destination, res);
            console.log("3")
        });

Test

async test(){
     console.log("1");
    const myDalle = await mapper.mapAsync({id: 1}, Dalle, IdOnlyDto);
    console.log("4");
}

Expected behavior Output : 1,2,3,4

Actual behavior Output : 1,2,4,3

llecorguille commented 2 years ago

Is it related to the "fake async" support ? (docs)

nartc commented 2 years ago

Fake async is using Promise.resolve() to schedule the task as a microtask, which should work correctly. I'll look into this. Thanks!

nartc commented 2 years ago

Well, turns out this has never worked correctly. I'll be investigating this further. But it's gonna be slow as I'm a bit busy these days.

micalevisk commented 2 years ago

With Mapper#map it should outputs 1,2,4,3
while with Mapper#mapAsync it should outputs 1,2,3,4 @llecorguille?

llecorguille commented 2 years ago

With Mapper#map it should outputs 1,2,4,3 while with Mapper#mapAsync it should outputs 1,2,3,4 @llecorguille?

Yes, that's it. But currently the Mapper#mapAsync outputs 1,2,4,3

nartc commented 2 years ago

This specific issue has been addressed with AutoMapper v8. However, the TRUE underlying issue with "Fake" async is still there. Complex async flow with AutoMapper isn't recommended for now. If you have a complex async flow to map from one object to another, do not use AutoMapper for that object pairs.