typeorm: Setting optional field to undefined and saving the object does not update database.

Issue type:

[ ] question [x] bug report [ ] feature request [ ] documentation issue

Database system/driver:

[ ] cordova [ ] mongodb [ ] mssql [ ] mysql / mariadb [ ] oracle [x] postgres [ ] sqlite [ ] sqljs [ ] react-native [ ] expo

TypeORM version:

[ ] latest [ ] @next [x] 0.2.7 (or put your version here)

Steps to reproduce or a small repository showing the problem:

Declare a model like

@Entity()
class Foo {
  ...
  @Column({ nullable: true })
  public field?: string;
}

Save an object with a value in the optional field.

const foo = new Foo();
foo.field = "something";
...
await repo.save(foo);

Try set the optional field as undefind, but the column is not nullified.

foo = await repo.findOne(...)
foo.field = undefined;
console.log(foo); // field is no longer in foo
await repo.save(foo); // results in an UPDATE sql statement that does not set 'field' to null

If replacing foo.field = undefined with foo.field = null, it works properly. However, it requires to use @ts-ignore to bypass the type check (assigning null to a string typed field).

I am wondering if this is the expected behaviour. Or am I doing anything wrong?

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 11
  • Comments: 18 (3 by maintainers)

Most upvoted comments

the reason why this is right:

is null is a valid data that you might want to persist to the database.

but undefined – should be considered a no-op.

In what scenario do you want a no-op? Or better said might be, can no-op’s be done a more intuitive way?

If I have a class/object I’m usually either inserting it new or updating it.

In either case I’m usually not going to have random fields I don’t want updated.

For TypeScript I’m going to design a class like:

interface User {
  id: string;
  username: string;
  nickname?: string; //optional
}

If the User wants to remove their nickname, I’d set it to undefined expecting it to be set to null in the database.

Having to go back and redeclare classes like this…

interface User {
  id: string;
  username: string;
  nickname?: string | null; //optional
}

…and change all the code to explicitly use null instead of the well established undefined pattern in TypeScript makes this library less attractive to use.

I’d rather see no-ops done using a different pattern. Like explicitly listing the fields you want to no-op when you call update.

I’d argue the no-op scenario is a bit of an edge case versus the pain we’re causing TypeScript users that are used to using undefined.

Setting field to undefined won’t change field value in database - it’s by design.

FWIW, that doesn’t work well with TypeScript where the standard is to use undefined over null for everything.

Is there a reason you designed it so undefined and null are different?

In mentioned issue when fetching values from columns with null in database we received undefined in our model. It was fixed. Setting field to undefined won’t change field value in database - it’s by design.

You can use the EntitySubscriberInterface to workaround this behavior. It’s not pretty, but it helps give a cleaner interface from a TypeScript perspective. This maps back and forth from null -> undefined on load/update.

@EventSubscriber()
export class NullToUndefinedSubscriber implements EntitySubscriberInterface {
  // Map null -> undefined so all accessors of this Entity can rely on undefined
  afterLoad?(_entity: any, event?: LoadEvent<any>) {
    if (event && event.entity) {
      const eventEntity = event.entity;
      for (const key of Reflect.ownKeys(eventEntity)) {
        const val = Reflect.get(eventEntity, key);
        if (val === null) {
          Reflect.set(eventEntity, key, undefined);
        }
      }
    }
  }

  // Do the inverse of above -- map undefined to null
  beforeUpdate?(event: UpdateEvent<any>) {
    if (event && event.entity) {
      const eventEntity = event.entity;
      for (const key of Reflect.ownKeys(eventEntity)) {
        const val = Reflect.get(eventEntity, key);
        if (val === undefined) {
          Reflect.set(eventEntity, key, null);
        }
      }
    }
  }
}

Hi there, I’m finding the hard way that ignoring ‘undefined’ during a .save() is the current behavior, which was not expected at all. Is that something that could be reconsidered in a future version of typeorm?

The reasoning is that it’s not the expected behavior for some subset of users, because it’s likely not the simplest representation when creating or loading a DB row into an Entity.

Ideally (like others suggested above), an Entity would alway be a full representation of a DB table row, with 1-1 mapping of (ts) fields to column, where undefined maps to NULL.

Use-cases where a user wants to update (or more generally manipulate) only a subset of columns of a table (ie a DB projection) can be handled through utility types such as Pick.

This could lead to safer code, as it’s more strongly typed, tell the programmer that they’re not working on the full entity/row, and doesn’t require custom mapping between null to undefined that might introduce further bugs through omission.

This is acceptable, as one can add a wrapper around this.

One cannot add a wrapper around this without forcing every usage of the optional value to also test for null. “null” is a frowned upon keyword in Typescript in favor of the “undefined” pattern. Declaring a value as nullable is required to support an optional value in the model. We should not also be forced to accept that null unless we declared as such.

I realize you’ve painted yourselves into a corner on this one. Is there an attribute we could put in the annotation for the optional fields that would allow TypeORM to treat an undefined field as same as null when saving to the DB? This would solve the problem for everyone.

Why should setting field to undefined update value to null in database? Undefined and null are two different types. You can define your field like this to use type checking:

public field?: string | null;

You are confusing the “null” type in Typescript with the NULL value in Postgres. NULL in SQL serves the same purpose as “undefined” in Typescript. null is an actual object, it isn’t NULL in SQL terms.

Because you have already implemented IGNORING undefined values, you should add an parameter we can set on optional values to treat undefined and NULL as equivalent.

@fchu I am not 100% certain with the following statements. So maybe I’m wrong, but I think the reason why undefined is ignored is because of relations. Look at the following entity:

tldr: property === undefined (not loaded relation) property === null (property was loaded and is set to Null in db)

@Entity()
export class Discount {
  @PrimaryGeneratedColumn()
  id!: number;

  @Column()
  code!: string;

  @ManyToOne((type) => Customer, { nullable: true })
  allowedCustomer?: Customer | null;

  @OneToOne((type) => PriceRule)
  @JoinColumn()
  priceRule?: PriceRule;

  @RelationId((discount: Discount) => discount.priceRule)
  priceRuleId!: number;
}

For all my relations I mark them as optional via ‘?’ and if they can be nullable I add ‘| null’, because if I don’t set relations on find/findOne options to [‘allowedCustomer’] this property could be undefined. Imagine now if undefined would lead to setting the db field to null. Then an unloaded relation could lead to unintentionally deleting some relations or am I wrong with that? In my opinion the distinction between null (db field is set to null) and undefined(relation was not loaded) is fine, because if I want to be sure if a relation is loaded I can add something like this:

if (this.allowedCustomer === undefined) throw new RelationNotLoadedError(Discount, 'allowedCustomer');

Why should setting field to undefined update value to null in database?

By doing foo.field = undefined, the foo object no longer has field property. But if we save the object to database, and retrieve the object back, the result will have field property, which seems inconsistent.

public field?: string | null;

This works for me.

Basically this issue is a duplicate of #584. If there is no intention/reason to make changes around this behaviour, this issue can be closed.

FYI, if we use mysql, we need to add type: 'time' to use Date | null type.

@Column({ nullable: true, type: 'datetime' })
deadline?: Date | null;

the reason why this is right:

is null is a valid data that you might want to persist to the database.

but undefined – should be considered a no-op.

This is acceptable, as one can add a wrapper around this.

Could this be documented, indeed?

Why should setting field to undefined update value to null in database? Undefined and null are two different types. You can define your field like this to use type checking:

public field?: string | null;