TsMorphMetadataProvider embedded array objects are not discovered after upgrade from 5.3.1 to 5.5.0

[QuestionAndAnswer]

Describe the bug

I'm getting the following error when initing MikroORM with TsMorph.

MetadataError: Entity 'PTE[]' was not discovered, please make sure to provide it in 'entities' array when initializing the ORM (used in BPE.titles)

While it is showing in debug mode that all files and both PTE and BPE entities was found, so entities is there.

Having the structure something like this

In P.ts

import { BPE } from "./BPE";

@Entity()
export class P {
  @PrimaryKey()
  id: string;

  @OneToOne(() => BPE, x => x.p, { ower: true, orphanRemoval: true,  })
  bp: IdentifiedReference<BPE> | null;
}

@Embeddable()
export class PTE {
   @PrimaryKey()
   id: string;

   @Property()
   name: string;
}

In BPE.ts

import { PTE, P } from "./P";

@Entity()
export class BPE {
  @PrimeryKey()
  id: string;

  @Embedded({ object: true })
  titles: PTE[];

  @OneToOne(() => P, x => x.bp, { orphanRemoval: true, eager: true })
  p: IdentifiedReference<P>;
}

Stack trace

MetadataError: Entity 'PTE []' was not discovered, please make sure to provide it in 'entities' array when initializing the ORM (used in BPE.titles)
    at Function.fromUnknownEntity (C:\Users\mr\Projects\pl\node_modules\@mikro-orm\core\errors.js:148:16)
    at MetadataDiscovery.initEmbeddables (C:\Users\mr\Projects\pl\node_modules\@mikro-orm\core\metadata\MetadataDiscovery.js:612:42)
    at C:\Users\mr\Projects\pl\node_modules\@mikro-orm\core\metadata\MetadataDiscovery.js:62:86
    at Array.forEach (<anonymous>)
    at C:\Users\mr\Projects\pl\node_modules\@mikro-orm\core\metadata\MetadataDiscovery.js:62:65
    at Array.forEach (<anonymous>)
    at MetadataDiscovery.processDiscoveredEntities (C:\Users\mr\Projects\pl\node_modules\@mikro-orm\core\metadata\MetadataDiscovery.js:62:18)
    at MetadataDiscovery.discover (C:\Users\mr\Projects\pl\node_modules\@mikro-orm\core\metadata\MetadataDiscovery.js:35:20)
    at async MikroORM.discoverEntities (C:\Users\mr\Projects\pl\node_modules\@mikro-orm\core\MikroORM.js:105:25)      
    at async Function.init (C:\Users\mr\Projects\pl\node_modules\@mikro-orm\core\MikroORM.js:50:9) {
  entity: undefined
}

To Reproduce
Try to generate a cache for example (without cache existing yet).
Or just call MikroOrm.init.

Expected behavior
No error happens, and discovery phase is passed.

Additional context
There were no such issue in 5.3.1, it happened straight after version up.
So, I've debugged the problem and I believe it is this commit which caused this issue.
4a69871

As basically next inside the call initEmbeddables here https://github.com/mikro-orm/mikro-orm/blob/master/packages/core/src/metadata/MetadataDiscovery.ts#L741

It is trying to find in metadata storage PTE[] string instead of PTE. PTE string EXISTS in metadata storage though! I've checked it. So, it is both written in console and in debug I found it.

Versions

Dependency	Version
node	16.17.0
typescript	4.7.3
mikro-orm	5.5.0
your-driver	postgresql

[B4nan]

Thanks for the repro and additional notes, wanna send a PR too? Should be just about stripping the [] from the end of the prop.type in the line you mentioned I guess.

[QuestionAndAnswer]

Well. if you saying about just to strip it - that's fine, I can send a fix.
But, I've attached this commit which deliberately added this a statement, and I don't really understand fully why it was added in the first place. Maybe there are other parts of the system which rely on this?

Because, looking into full scope of this code, it is looking strange. First it was removed, but than it was added back if it is array.

type = type
      .replace(/Array<(.*)>/, '$1') // unwrap array
      .replace(/\[]$/, '')          // remove array suffix
      .replace(/\((.*)\)/, '$1');   // unwrap union types

    if (prop.array && !type.endsWith('[]') && !type.includes(' | ')) {
      type += '[]';
    }

I'll try, but no promises.

[QuestionAndAnswer]

Or you mean strip the line exactly for embeddable init?

[B4nan]

That code is doing a cleanup of possibly complex type expressions and tries to normalize it first. It strips the [] so it can strip the parens afterwards more predictably (at least I think so, its been some time I wrote it). But this information was required in other places.

Maybe we don't need to strip it in the first place, but we would still need to conditionally add it as it might not be there at all, e.g. with the Array<T>. Maybe it would be enough to set prop.array = true instead. Unfortunately I've been lazy to add any test case so hard to say - and therefore I'd rather not change it and fix it in the discovery as already suggested.

edit: or maybe some tests would fail if we revert it, feel free to try

[B4nan]

I think it's about this place:

https://github.com/mikro-orm/mikro-orm/blob/master/packages/core/src/metadata/MetadataDiscovery.ts#L952-L994

Scalar properties still need to be marked as arrays and that's where this information was needed. So we'd have to check for prop.array too if we want to use that instead of appending [] to the type.

[QuestionAndAnswer]

Well. Tests on the master branch I've pulled are not all passing on windows, because of basic differences in the path separator and other minor things. So, it will take a time.

[B4nan]

I believe it works fine via WLS, it did last time I tried (few months ago).