Non-static update method of the Models returns old value

[BjarkeNL]

Hi! This library looks like really cool - this is a much better way to declare model-classes for sequelize! Unfortunately i ran into a problem when trying it out:

Versions

• sequelize@4.43.2

• sequelize-typescript@0.6.10

• typescript@3.4.5

• reflect-metadata@0.1.13

• @babel/core@7.4.4

• @babel/preset-typescript@7.3.3

• @babel/plugin-proposal-class-properties@7.4.4

• @babel/plugin-proposal-decorators@7.4.4

I'm submitting a ...
[x] bug report
[ ] feature request

Actual behavior:

I tried converting my existing sequelize code to sequelize-typescript. It worked for the most part but i encountered a difference in behavior.
(Not 100% how to debug this, so i am reporting it as a bug here - could of course be a configuration issue or similar on my end)

The non-static update-function on a Model, normally returns a promise containing a model-object with the updated property values. After changing from plain sequelize to sequelize-typescript, it no longer does this - instead the promise now contains a model-object with the old values from before the update.
(The database is updated with the new values, the problem is just that the return value of the update-method contains different values than it normally does)

When debugging, i can see that the dataValues-object of the model-object are updated with the new values, but the properties of the model-object still contain the old values. (They are not updated)

The same problem happens for the with set + save combo.

Expected behavior:

The non-static update method should return a model-object with updated values, like in plain sequelize. (and same for set+save)

Steps to reproduce:
See code below

Related code:

The model:

import {Table, Column, Model, HasMany, DataType, BelongsTo, BeforeDestroy, DeletedAt, UpdatedAt, CreatedAt} from 'sequelize-typescript';

@Table({
    tableName: 'users',
    timestamps: true,
    underscored: true,
    paranoid: true
})
export default class User extends Model<User> {

    @Column({type: DataType.INTEGER, field: "id", primaryKey: true, autoIncrement:true})
    id: number = this.id; // <- see: https://github.com/sequelize/sequelize/issues/10579  https://github.com/babel/babel/issues/9105
       
    @Column({type: DataType.STRING(100), field: "first_name", allowNull: false})
    firstName: string = this.firstName;
}

The code to reproduce the problem:

...

const user = await User.findById(userId, { transaction: t });

console.log("Before update: ", user.firstName); // the old value

const updatedUser = await user.update({ firstName: "Something" }, { transaction: t });

console.log("After update: ", updatedUser.firstName); // still the old value

...

Same problem happens with:

...

const user = await User.findById(userId, { transaction: t });

console.log("Before update: ", user.firstName); // the old value

user.set({ firstName: "Something" });

console.log("After set: ", user.firstName); // still the old value

const updatedUser = await user.save({ transaction: t });	

console.log("After save: ", user.firstName); // still the old value
console.log("After save: ", updatedUser.firstName); // still the old value

...

Without sequelize-typescript (i.e. with just plain sequelize without typescript), it works.

It is of course not impossible that it is a babel vs typescript issue.
Could it, for example, be because i'm assigning each property to itself in the model? (for the reason for doing this, see: sequelize/sequelize#10579 and babel/babel#9105 ).

Library configurations:

.babelrc:

{
    "presets": [
	["@babel/env", {"targets": {"node": "8.0"}}],
	"@babel/react",
	["@babel/typescript", {"isTSX": true, "allExtensions": true}]
    ],
    "plugins": [
	["@babel/plugin-proposal-decorators", { "legacy": true }],
	["@babel/plugin-proposal-class-properties", { "loose":  true }]
    ]
}

tsconfig.json:

{
    "compilerOptions": {
	"target": "esnext",
	"moduleResolution": "node",
	"allowJs": true,
	"noEmit": true,
	"strict": true,
	"isolatedModules": true,
	"esModuleInterop": true,

	"jsx": "react",

	"experimentalDecorators": true,  // for sequelize-typescript
	"emitDecoratorMetadata": true   // for sequelize-typescript
    },
    "include": [
	"./src/"
    ]
}

Sequelize-typesctipt setup:

import config from ...;
import {Sequelize} from 'sequelize-typescript';

const sequelize = new Sequelize({
    database: config.db.database,
    username: config.db.username,
    password: config.db.password,
    host: config.db.host,
    port: config.db.port,
    dialect: "mysql",
    pool: {
	max: 100,
	min: 0,
	idle: 1000000
    },
    define: {
	charset: 'utf8mb4',
	collate: 'utf8mb4_unicode_ci',
	timestamps: true
    },
    logging: config.sequelize_logging ? (x: any) => console.log("[SEQUELIZE]",x) : () => false,
    typeValidation:false,
    modelPaths: [__dirname + "/../entities"]
});


sequelize
    .authenticate()
    .catch(err => {
	console.error('Unable to connect to the database:', err);
    });

sequelize.sync({
    force: false
});

export default sequelize;

insert short code snippets here

[RobinBuschmann]

Hey @BjarkeNL, I think the issue is indeed caused by this behaviour of babel. I've got the impression that _defineProperty overrides the previously defined getters and setters created by sequelize. As you already mentioned the dataValues are updated. But when calling user.firstName it uses the getter which is defined by the babel plugin within the constructor of User and in turn doesn't get through the dateValues. So to me it definitely looks like an issue with babel-plugin-proposal-class-properties. Can you omit this plugin? If not, I'm trying to think about a workaround.

Hope this helps!

[Telokis]

@RobinBuschmann Hey! I have this exact problem. Did you manage to find a solution?

[loganfsmyth]

This is a current difference in behavior between Typescript and ECMAScript class fields. Babel's TypeScript transformation parses TS, but the runtime behavior for ECMA features follows the ECMA spec, not TS.

microsoft/TypeScript#27644

[RobinBuschmann]

@loganfsmyth thanks for giving more insights. So babel-plugin-proposal-class-properties is doing it the right way 🤔

@Telokis @BjarkeNL
For now I can only provide a workaround (Overriding the defined properties again):

function Fix(target): void {
  return class extends target {
    constructor(...args: any[]) {
      super(...args);
      Object.keys(new.target.rawAttributes).forEach(propertyKey => {
        Object.defineProperty(this, propertyKey, {
          get() {
            return this.getDataValue(propertyKey);
          },
          set(value) {
            this.setDataValue(propertyKey, value);
          }
        });
      });
    }
  } as any;
}

@Table
@Fix
export class Actor extends Model<Actor> { }

[DalerAkhmetov]

@RobinBuschmann, this solution doesn't work with associations.

@Table
@Fix
export default class Example extends Model<Example> {
  @AllowNull(false)
  @ForeignKey(() => SomeModel)
  @Column(DataType.INTEGER)
  someModelId!: number;

  @BelongsTo(() => SomeModel)
  someModel!: SomeModel;
}

example.someModel is undefined where example is instance of Example.

[RobinBuschmann]

@DalerAkhmetov Good point. One need to add the association keys as well like so:

[
  ...Object.keys(new.target.rawAttributes), 
  ...Object.keys(new.target.associations),
].forEach(propertyKey => {
  // ..
});

(Untested, but) Hope this helps for now!

[DalerAkhmetov]

@DalerAkhmetov Good point. One need to add the association keys as well like so:

[
  ...Object.keys(new.target.rawAttributes), 
  ...Object.keys(new.target.associations),
].forEach(propertyKey => {
  // ..
});

(Untested, but) Hope this helps for now!

thanks, it works

[DalerAkhmetov]

Hi @RobinBuschmann, I found a bug. Solution #612 (comment) doesn't work if I use bulkCreate and pass records as array of Model instance
For example:

let invoices = [];
invoices.push(Invoice.build({amount: 100}));
invoices.push(Invoice.build({amount: 120}));
invoices.push(Invoice.build({amount: 150}));

await Invoice.bulkCreate(invoices); // UNEXPECTED BEHAVIOUR - bulkCreate will try to insert records without fields (in our case it's amount);
// but the following line works fine
await Invoice.bulkCreate(invoices.map(e => e.get({plain: true})));

I don't know how to fix it. Do you have any ideas?

[tommoor]

@DalerAkhmetov Good point. One need to add the association keys as well like so:

[
  ...Object.keys(new.target.rawAttributes), 
  ...Object.keys(new.target.associations),
].forEach(propertyKey => {
  // ..
});

This is awfully close but it does seem to result in any modified associations being included in the models internal changed() calculation. for example if you have a BelongsTo association setup for collection, and then do something like:

const item = new Item();
item.collection = new Collection();
item.save()

The query that's generated is invalid because it tries to save the collection field. I don't yet have a solution, suspect I need to understand sequelize internals much more fully.

---

Edit: This solution also overrides any existing custom getter/setters 😢

---

Edit 2: You can find a working implementation of the Fix decorator here: https://github.com/outline/outline/blob/0de6650aa5e97612d46b459de719a000da6349be/server/models/decorators/Fix.ts#L9

[EdisonJpp]

I'm getting the same error and I've not fixed it yet. I tried all the above solutions and it does not work for me :'(