-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
Description
Issue description
Embedded Entities do not respect entitySkipConstructor
Expected Behavior
When entitySkipConstructor is set to true, embedded entities should also bypass their constructors on hydration. Without this behavior, it becomes impossible to model values as embedded entities with constructors.
Actual Behavior
When entitySkipConstructor is set to true, embedded entity constructors are still called. This is because of the following code in EmbeddedMetadata:
create(options?: { fromDeserializer?: boolean }): any {
if (!(typeof this.type === "function")) {
return {}
}
if (!options?.fromDeserializer || this.isAlwaysUsingConstructor) {
return new (this.type as any)()
} else {
return Object.create(this.type.prototype)
}
}Steps to reproduce
set entitySkipConstructor to true in typeorm.
Then create an entity like:
class Address {
constructor(value: string) {
// do something that requires `value` to be set
}
}
@Entity()
class Person {
@Column(() => Address)
address: Address;
constructor(address: Address) {
this.address = address;
}
}The constructor of Address will be called on hydration and if the constructor will fail because the params are not passed in, the entity will fail to construct and exceptions will be thrown.
My Environment
| Dependency | Version |
|---|---|
| Operating System | |
| Node.js version | x.y.zzz |
| Typescript version | x.y.zzz |
| TypeORM version | x.y.zzz |
Additional Context
I have done a quick test, and it appears that the following changes (flipping the conditionals) still passes all tests and allows for the expected behavior:
create(options?: { fromDeserializer?: boolean }): any {
if (!(typeof this.type === "function")) {
return {}
}
if (options?.fromDeserializer || !this.isAlwaysUsingConstructor) {
return Object.create(this.type.prototype)
} else {
return new (this.type as any)()
}
}I will open a PR to fully test this (haven't gotten the tests working on my m1 mac).
Relevant Database Driver(s)
- aurora-mysql
- aurora-postgres
- better-sqlite3
- cockroachdb
- cordova
- expo
- mongodb
- mysql
- nativescript
- oracle
- postgres
- react-native
- sap
- spanner
- sqlite
- sqlite-abstract
- sqljs
- sqlserver
Are you willing to resolve this issue by submitting a Pull Request?
Yes, I have the time, and I know how to start.