Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EntityFactory.create fails if the entity class constructor uses destructuring assignment. #781

Closed
rangelfinal opened this issue Aug 25, 2020 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@rangelfinal
Copy link

Describe the bug
When using destructuring assignment on a entity class constructor, meta.constructorParams is set as [undefined] on EntitySchema.setClass, which is reflected as extractConstructorParams returning [undefined] on EntityFactory.createEntity and the following return new Entity(...params); failing with a TypeError.

this._meta.constructorParams = Utils.getParamNames(proto, 'constructor');

const params = this.extractConstructorParams<T>(meta, data);
meta.constructorParams.forEach(prop => delete data[prop]);
// creates new instance via constructor as this is the new entity
return new Entity(...params);

That seems to happen when Utils.getParamNames gets to the case 'BindingElement': return p.left.name;, and p.left.name is undefined as the destructuring assignment parameter is unamed.
case 'BindingElement':
return p.left.name;

A possible fix would be to check if p.left is defined but p.left.name is undefined - then it's probably a destructuring assignment and the property names should be found on p.left.properties.

To Reproduce

import { Entity, PrimaryKey } from '@mikro-orm/core';
import { v4 } from 'uuid';

@Entity()
class User {
   @PrimaryKey()
  id? = v4();

  constructor({ id }: Partial<User>) {
    this.id = id;
  }
}

Calls like em.getRepository(User).create({ id: ' fake-uuid-like-string' }) will fail with Cannot destructure property 'id' of 'undefined' as it is undefined.

Versions

Dependency Version
node v12.16.3
typescript 3.9.7
@mikro-orm/core 4.0.0-rc.4
@mikro-orm/postgresql 4.0.0-rc.4
@rangelfinal rangelfinal added the bug Something isn't working label Aug 25, 2020
@B4nan
Copy link
Member

B4nan commented Aug 27, 2020

This use case was not really supported, and the error is not caused by that undefined in ctor params, but rather in your ctor implementation, as you rely on the ctor value, but ORM won't give you any if the parameter is not named as one of the properties.

I was recently thinking about this, and I think we should provide the full data object in the ctor param if there is something either not matching existing properties or not having a name (like your example), which should help in most cases.

Btw the reproduction like this is not really reproducing anything, as EntityFactory will never call the ctor if you provide all the PKs. Not talking about using initializer for the id property, as that will override the id from ctor anyway. To make the test really fail, I did add name: string property and used that in the ctor instead of id.

@B4nan B4nan closed this as completed in 06a5490 Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants