5.6.16 fails with Cannot read properties of undefined (reading '__helper')

[strmer15]

Describe the bug

After updating to MikroORM 5.6.16, our tests are now failing with Cannot read properties of undefined (reading '__helper'). When pinning back to 5.6.15, everything works as expected.

Stack trace

TypeError: Cannot read properties of undefined (reading '__helper')

      258 |     };
      259 |
    > 260 |     const [queryResults, totalCount] = await this.entityRepository.findAndCount(
          |                                        ^
      261 |       filterClauses,
      262 |       {
      263 |         limit,

      at helper (../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/entity/wrap.js:21:19)
      at ../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/entity/EntityLoader.js:185:47
          at Array.forEach (<anonymous>)
      at EntityLoader.initializeOneToMany (../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/entity/EntityLoader.js:183:22)
      at EntityLoader.initializeCollections (../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/entity/EntityLoader.js:162:18)
      at EntityLoader.populateMany (../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/entity/EntityLoader.js:157:14)
      at EntityLoader.populateField (../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/entity/EntityLoader.js:241:27)
      at EntityLoader.populate (../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/entity/EntityLoader.js:43:13)
      at SqlEntityManager.find (../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/EntityManager.js:137:9)
          at async Promise.all (index 0)
      at SqlEntityManager.findAndCount (../../../node_modules/.pnpm/@mikro-orm+core@5.6.16_fb774276160e13cf5ebe03ab217887af/node_modules/@mikro-orm/core/EntityManager.js:268:35)
      at OrderRepository.find (src/repositories/order.ts:260:40)
      at Object.<anonymous> (src/repositories/order.medium-test.ts:831:38)

To Reproduce
Steps to reproduce the behavior:

1. Create a OneToMany and a ManyToOne entity relationship, e.g. Order and LineItem
2. Attempt to query by the parent, either a single item or a collection, e.g. Order
3. Throws an error

Expected behavior

MikroORM allows the entities to be queried.

Additional context

We actually have two OneToMany / ManyToOne that use the same child model, e.g. from Order->LineItem and Shipment->LineItem. Here's some of declarations:

In the Order entity:

  @OneToMany({
    entity: 'LineItem',
    mappedBy: 'order',
  })
  lineItems = new Collection<LineItem>(this);

  @OneToMany({ entity: 'Shipment', mappedBy: 'order' })
  shipments = new Collection<Shipment>(this);

In the Shipment entity:

  @OneToMany({
    entity: 'LineItem',
    mappedBy: 'shipment',
  })
  lineItems = new Collection<LineItem>(this);

  @ManyToOne(() => Order, { hidden: true })
  order!: Order;

In the LineItem entity:

  @ManyToOne(() => Order, { hidden: true })
  order!: Ref<Order>;

  @ManyToOne(() => Shipment, { hidden: true })
  shipment!: Ref<Shipment>;

Not sure if that matters, I haven't been able to find a workaround for this problem, so I'm not sure what's relevant or not.

Versions

Dependency	Version
node	18.15.0
typescript	5.0.4
mikro-orm	5.6.16
your-driver	pg

[B4nan]

I need a complete reproduction for such issues.

Note that you should have ref: true in the decorator options, unless you are using ts-morph:

  @ManyToOne(() => Order, { hidden: true, ref: true })
  order!: Ref<Order>;

But that's probably not related.

[strmer15]

Thanks for the quick response!

I need a complete reproduction for such issues.

I'll see if I can work on that in the next couple of days, but I might not get to that until next week.

Note that you should have ref: true in the decorator options

Ok, I gave that a shot, but it didn't seem to have any effect that I could see in the tests. 🤷

[B4nan]

It comes from this commit most probably:

875d966

But it sounds really weird that the value wouldn't be there, so without a failing repro I don't know what to fix.

[strmer15]

@B4nan I've uploaded a reproduction that's failing on 5.6.16 at https://github.com/strmer15/MikroOrmBug - let me know if you have any trouble using it to reproduce the problem. Thanks again!

[B4nan]

Weird, I can't reproduce this in the ORM codebase, but I see your repro failing. Even when I simplified the setup to use sqlite and put everything into one file - the same code copied to the ORM test case is passing, even though they use the exact same version (just updated to the very last dev one).

[B4nan]

Ok, found it, this is about your TS config, you use ES2022 as the target, which has enabled useDefineForClassFields option - and that breaks propagation, as the ORM is no longer able to redefine the class properties with get/set pairs. There are places the ORM works with the underlying backing store directly to speed up things, and this was one of them.

You have two options, disable that in your tsconfig or use declare for your entity properties (both have the same effect, one is global, one is local). I will keep this open and try to find a way to make things work with useDefineForClassFields enabled, as there will be more and more people falling into this trap. Worst case will add a warning to the docs somewhere.

[strmer15]

Thanks @B4nan ! I'll try using the declare syntax and see if we can get it working that way. I'd prefer not removing the ES2022 target since everything's on Node 18 now in our codebase.

[B4nan]

Thanks @B4nan ! I'll try using the declare syntax and see if we can get it working that way. I'd prefer not removing the ES2022 target since everything's on Node 18 now in our codebase.

The target itself is not a problem, it's only about useDefineForClassFields whose default value has changed in ES2022+. You can still use ES2022 and just disable useDefineForClassFields in your compiler options.

[strmer15]

Oh, gotcha - yeah, I'll just disable it in our compiler options, that would be a lot simpler 😁

[parisholley]

this issue (useDefineForClassFields defaulting to true in latest JS) burned an entire day of debugging for me (not sure why I didn't google this error message first, lol). in chance of adding some type of validation (either runtime or perhaps as a post-installs script) to check for that so others don't get burned?

[B4nan]

I think I know a way to get around this, although it has a measurable perf impact, so doing some smart detection during discovery and making it conditionally would be best. We need to first call delete on the properties after the class instance is created (as the definedProperty calls are done in the constructor), then define them again with our own implementation that supports propagation, etc, and then we need to reassign the values to trigger the propagation and actually store the values in __data.

Since we will be already detecting it, we can start with a warning during discovery and provide a fallback later. Not sure how easy this will be to detect, I'd rather not read stuff from tsconfig, as the same problem could be there on compiled output. Maybe checking the entities provided by user, if we see a relation property, we can be quite sure it should store data in __data, if that won't work, we know it's time to warn about not functional propagation.

[B4nan]

FYI with #4730 merged, v6 should work fine with the useDefineForClassFields option enabled.