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

Embedded Entities do not respect entitySkipConstructor #9824

Closed
18 tasks
themizzi opened this issue Mar 3, 2023 · 0 comments · Fixed by #9831
Closed
18 tasks

Embedded Entities do not respect entitySkipConstructor #9824

themizzi opened this issue Mar 3, 2023 · 0 comments · Fixed by #9831

Comments

@themizzi
Copy link
Contributor

themizzi commented Mar 3, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant