Map To PK feature does not convert Id to custom type

[thiagomini]

Describe the bug

• Given we have a relationship of One-to-Many: 1 User has Many Profiles
• And the Profile side uses the mapToPk: true feature
• And the User side's Id is a custom type (a Value Object)
• When we fetch an existing Profile entity (Many side) from the database
• Then the Profile's userId attribute is a plain string

Stack trace

 FAIL  src/mikro-orm.test.ts
  Mikro Orm
    ✕ A profile user id should be a custom type (42 ms)

  ● Mikro Orm › A profile user id should be a custom type

    expect(received).toBeInstanceOf(expected)

    Expected constructor: Id

    Received value has no prototype
    Received value: "1"

      58 |     
      59 |     expect(userProfile).toBeTruthy();
    > 60 |     expect(userProfile.user).toBeInstanceOf(Id)
         |                              ^
      61 |   })
      62 |
      63 | });

      at Object.<anonymous> (src/mikro-orm.test.ts:60:30)
...

To Reproduce
Steps to reproduce the behavior:

1. Clone the minimum reproducible code repository:https://github.com/thiagomini/mikro-orm-repro/tree/bug/map-to-pk
2. run yarn
3. run docker compose up -d
4. run yarn test

Expected behavior
the userId should be an instance of Id instead of a plain string.

Additional context
The id data type in the database is actually a bigint

Versions

Dependency	Version
node	18.12.1
typescript	5.2.2
mikro-orm	5.8.7
pg	8.11.2

[thiagomini]

I already pinned the offender line, @B4nan, it's within the ObjectHidrator:

mikro-orm/packages/core/src/hydration/ObjectHydrator.ts

Line 29 in 20179ec

Utils.callCompiledFunction(hydrate, entity, data, factory, newEntity, convertCustomTypes, schema);

Right before executing this line, the entity.user is a getter which returns the correct Id instance. However, this line of the compiled function turns that into the "DatabaseValue":

if (data.user != null && convertCustomTypes) {
    data.user = convertToDatabaseValue_user(entity.user);
 }

Then, the databaseValue is indeed a string, because that's the Database representation of the custom type. However, why is it casting the value to a database representation when we just retrieve it from the database? It should do the opposite, right?

⚠️ Edit: I believe this is the line that creates that part of the compiled function:

mikro-orm/packages/core/src/hydration/ObjectHydrator.ts

Line 167 in 20179ec

ret.push(` if (data${dataKey} != null && convertCustomTypes) {`);

Could you give me any help here on how to fix that, Martin? thanks!

[thiagomini]

Hey again, Martin. I believe I found out a way to fix that - I'll create a PR so you can review it. Basically, I've added modified one in the hydrateOne callback:

if (prop.mapToPk) {
                ret.push(`      entity${entityKey} = convertCustomTypes ? convertToJSValue_${convertorKey}(data${dataKey}) : data${dataKey};`);
            }

So, if we have the convertCustomTypes flag as true, we try to convert the value to its JS representation first before assigning it to the entity.

[B4nan]

I already have a (much simpler and more correct) fix, give me a sec.

[thiagomini]

Sure, Martin, I appreciate it!

[thiagomini]

Hey again, @B4nan , thanks for the fix! Could you publish the latest change? I believe the publish step wasn't triggered because we had one failing test in the ci: https://github.com/mikro-orm/mikro-orm/actions/runs/6471482172/job/17570001107

[B4nan]

Those flaky tests...