Request to Revert Recent Changes made in #10289

I would like to request a reconsideration of the recent changes made in #10289. In my view, these modifications have negative consequences.

Maintainability

Maintainability of code suffers from this change. Modifying embedded entities results in additional effort to update client code associated with the entity. To illustrate this issue consider the following example:

class Company {
  @Column({ type: 'varchar' })
  name: string

  @Column({ type: 'varchar', nullable: true })
  address: string | null
}

export class Payroll {
  @PrimaryGeneratedColumn('uuid')
  uuid: string

  @Column(() => Company)
  company: Company
}

Now consider that some change requests that the company name need not be filled in initially:

class Company {
  @Column({ type: 'varchar', nullable: true })
  name: string | null

  @Column({ type: 'varchar', nullable: true })
  address: string | null
}

This forces us to modify the payroll class as well.

export class Payroll {
  @PrimaryGeneratedColumn('uuid')
  uuid: string

  @Column(() => Company)
  company: Company | null
}

I hope it is clear from this toy example that a change in the internal structure of the Company class suddenly forces changes throughout the codebase because the newly added behaviour will set company to null instead of a Company object with null fields. The changes in this example are limited to only the Payroll class, but all client code of the Payroll class is suddenly forced to perform null checks wherever it accesses the Company class.

A single change to make a field nullable will have a ripple effect throughout the entire codebase. If the change is not dealt with properly it will instead lead to unexpected failures where an object was previously expected instead of null.

Code semantics

The introduced changes hurt the semantics of code. Explicitly marking a value as null signifies an optional or missing value. Defaulting an object with persisted null properties to null instead of an object with null properties means the semantic meaning of a non-optional value has been lost.

Additionally, an object with all nullable properties still has reason to exist when the object or class has behaviour defined on it, which takes into account the fact that the properties of that class are optional.

Breaking changes

Existing codebases which currently use embedded entities with only nullable properties will need to be reworked. When the Typeorm version that existing codebases use is increased without great care (i.e. without knowing about these changes), it will break the inner workings of that project. Existing projects have (unknown) assumptions on how Typeorm deals with embedded entities.

The impact of theses changes will slow down the adoption of these and future changes because large existing codebases need to invest a considerable amount of time into the rework. I would assume that it will also encourage some to look towards other ORMs.