mikro-orm icon indicating copy to clipboard operation
mikro-orm copied to clipboard

Polymorphic Embeddables in combination with embedded properties

Open dehrhard opened this issue 3 years ago • 6 comments

Describe the bug I have an issue when using Polymorphic Embeddables in combination with nested embedded properties. When two Polymorphs have a property with the same name their type is conflicting and ending mixed up. I think I traced the issue to this spot in https://github.com/mikro-orm/mikro-orm/blob/473795c26f0b54b3997ce4c839a2834ff43290d8/packages/core/src/metadata/MetadataDiscovery.ts#L661

polymorphs.forEach(meta => {
  Object.values(meta.properties).forEach(prop => properties[prop.name] = prop);
  processExtensions(meta);
});

The properties Object receives the properties from both polymorhps. If multiple polymorphs have the same property name, the latter overwrites the former.

Example Consider the shortened example from the Docs

export enum AnimalType {
  CAT,
  DOG,
}

@Embeddable()
export class CatFood {

  @Property()
  mice = 2;
}

@Embeddable()
export class DogFood {

  @Property()
  cats = 10;
}

@Embeddable({ abstract: true, discriminatorColumn: 'type' })
export abstract class Animal {
  @Enum()
  type!: AnimalType;

  @Property()
  name!: string;
}

@Embeddable({ discriminatorValue: AnimalType.CAT })
export class Cat extends Animal {

  @Property()
  canMeow? = true;

  @Embedded()
  food: CatFood;
}

@Embeddable({ discriminatorValue: AnimalType.DOG })
export class Dog extends Animal {

  @Property()
  canBark? = true;

  @Embedded()
  food: DogFood;
}

@Entity()
export class Owner {
  @PrimaryKey()
  id!: number;

  @Property()
  name!: string;

  @Embedded()
  pet!: Cat | Dog;
}

In this case the food property of class Cat/Dog is ambiguos and depending on which polymorph is processed first, food might receive type Dogfood for Polymorph Cat or receive type Catfood for Polymorph Dog

Expected behavior Depeding on the used polymorph, the property should receive the associated type

Versions

Dependency Version
node 16
typescript 4.6.2
mikro-orm 5.0.4
@mikro-orm/postgresql 5.0.4

dehrhard avatar Apr 05 '22 14:04 dehrhard

I was just about to open a new issue regarding this same issue. Any news here, since this is now 2 years old?

Just wanted to provide my reproduction anyway and raise awareness that this issue still exists: https://github.com/NicklasKnell/reproduction/tree/polymorphic-embeddable

NicklasKnell avatar May 31 '24 11:05 NicklasKnell

+1

devcaeg avatar Jun 11 '24 04:06 devcaeg

I created a test in my fork for this, but I am still getting to know MikroOrm, as I have never built anyhting like it.

https://github.com/NicklasKnell/mikro-orm/blob/00a18bcf269206e073f162f052fb5d13e8eaf0c5/tests/features/embeddables/GH2987.test.ts

NicklasKnell avatar Jun 11 '24 14:06 NicklasKnell

I'm seeing this too. I'm doing something like pet!: Animal;

and

class Animal {
@ManyToOne()
food: Food
}

or

class Cat extends Animal {
@ManyToOne()
food: CatFood
}

but haven't found a good solution.

Polymorphic relations are pretty much essential eventually in any real project, and Mikro-ORM is so close - I'd love to see it provide good support.

buzzware avatar Jun 13 '24 09:06 buzzware

@buzzware so the solution that we are using for now looks something like this:

class Animal {}

class Cat extends Animal {
  catFood: CatFood
}

class Dog extends Animal {
  dogFood: DogFood
}

And while working with those Embeddables we assert the types when we need to access the food properties.

If needed I could also provide a more in-depth example.

NicklasKnell avatar Jun 13 '24 11:06 NicklasKnell

I spent all day yesterday trying to make it work where the embeddable contains a ManyToOne relation, and then do a find with an object specifying the embedded relation, but it gave the error you started the issue with - confusing the two embeddables.

buzzware avatar Jun 14 '24 01:06 buzzware

Some bugs are simple, some are not - this one was the latter. But #5784 is finally here. It will need more work (and maybe even a bit different approach), but it will most probably land in v6.3.

B4nan avatar Jul 08 '24 07:07 B4nan

@B4nan Excellent - The promise of polymorphism was a factor in me choosing Mikro-ORM. I have a background in Ruby on Rails, and so while this is a good step, I still hope for something like :

class Tag {
  id: number
  name: string
}

class Tagging {
  id: number
  tag: Tag
  // this would use subject_type and subject_id columns to select any animal subclass instance but we could also specify object to allow any class
  @ManyToOne({polymorphic: true, discriminatorColumn: 'type'})
  subject: Animal
}

Many thanks for this library and continual improvements.

buzzware avatar Jul 08 '24 09:07 buzzware

Well, that's a completely different feature #706.

B4nan avatar Jul 08 '24 09:07 B4nan