Column change detection for json(b) columns with transformer maybe wrong

[spalberg]

Issue type:

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

Database system/driver:

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

TypeORM version:

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

Steps to reproduce or a small repository showing the problem:

I have an entity containing monetary values that I want to manage via dinero.js. Dinero provides a toObject function to output some json. This json I wanted to persist in the database and use a value transformer to automatically switch between Dinero-instance and json. The entity in question looks like this:

export class Article extends BaseEntity {
  // ...
  @Column({
    nullable: false,
    transformer: new MonetaryTransformer(),
    type: 'json',
  })
  price: Dinero.Dinero;
}

class MonetaryTransformer {
  to(value) {
    return value.toObject();
  }
  from(value) {
    return Dinero(value);
  }
}

However updates of price to an existing entity of Article are not persisted. Changes to other fields are persisted, just not price changes.

The reason seems to be the change detection algorithm in SubjectChangedColumnsComputer.ts (https://github.com/typeorm/typeorm/blob/master/src/persistence/SubjectChangedColumnsComputer.ts#L90). For columns of type json or jsonb a deepCompare is performed. This deepCompare always returns true even for different Dinero-instances (see https://codesandbox.io/s/nice-merkle-st0f0). This results in the pricecolumn always beeing "unchanged" in the eyes of typeorm.

My question is: Is this the intended behavior? The comparison is based on the application-values instead of the database-values. This seems wrong. I think that if the column is json(b) and there is a transformer present, entityValue and databaseValue (https://github.com/typeorm/typeorm/blob/master/src/persistence/SubjectChangedColumnsComputer.ts#L90) should be transformed using the transformers to-method before comparison.

My workaround for now is making the column text and wrapping the transformer returns in JSON.stringify/parse.

[chetanism]

+1