.toJSON not including getterMethods

[W0mpRat]

Recently upgraded to v5 from v4. Version 5.8.6

User model with fullName getterMethod

module.exports = (sequelize, DataTypes) => {
  const User = sequelize.define('User', {
    firstName: {
      type: DataTypes.STRING,
      allowNull: false
    },
    lastName: {
      type: DataTypes.STRING,
      allowNull: false
    },
    email: {
      type: DataTypes.STRING,
      allowNull: false,
      unique: true
    },
    password: {
      type: DataTypes.STRING,
      allowNull: false
    }
  }, {
    getterMethods: {
      // TODO: Not working in latest sequelize
      fullName () {
        const fullName = `${this.firstName} ${this.lastName}`

        return fullName
      }
    },
    paranoid: true
  })

  User.associate = (models) => {
    User.belongsToMany(models.Project, {
      through: models.UserProject,
      foreignKey: 'userId'
    })
    User.belongsTo(models.Company, {
      foreignKey: 'companyId',
      onDelete: 'CASCADE'
    })
  }

  return User
}

Controller endpoint:

  async show (req, res) {
    try {
      const result = await User.findByPk(req.params.id, {
        include: [{
          through: {
            attributes: ['default']
          },
          model: Project
        }]
      })

      res.status(200).send(result)
    } catch (error) {
      sendTeamsError(error)
      res.status(400).send(error)
    }
  },

Bug:

result has fullName property but result.toJSON() does not include fullName

[pi-kei]

That's because of this breaking change #9568
Exactly because of this new if-statement

sequelize/lib/model.js

Lines 3318 to 3323 in 2c50b7e

	if (
	this._options.attributes
	&& !this._options.attributes.includes(_key)
	) {
	continue;
	}

If you add VIRTUAL field fullName instead of getter, then request it in attributes ['firstName', 'lastName', 'fullName'], it would work.
But with getter it would fail.

[mjy78]

I've noticed if you add a VIRTUAL field fullName with no get function defined, then the getterMethod appears to work....

const User = sequelize.define('User', {
  firstName: {
    type: DataTypes.STRING,
    allowNull: false
  },
  lastName: {
    type: DataTypes.STRING,
    allowNull: false
  },
  fullName: {
    type: DataTypes.VIRTUAL
  }    
  }, {
  getterMethods: {
    fullName () {
      return `${this.firstName} ${this.lastName}`;
    }
  }
})

Not sure if this is good/recommended practice?

[papb]

@mjy78 Once this gets fixed, it won't be a good practice. But for now it's a valid workaround, I'd say. Stay tuned.

[jayarjo]

@papb and how it is going to work, once it's fixed?

@mjy78 does it work if you have get on the VIRTUAL field and no corresponding getterMethod?

[papb]

@papb and how it is going to work, once it's fixed?

@jayarjo When I self-assigned this, my idea was to make every getterMethod appear in the .toJSON() output, just that, very simple. But now I realize this might be technically considered a breaking change...

@mjy78 does it work if you have get on the VIRTUAL field and no corresponding getterMethod?

Yes, it works. I personally prefer this way, even if the other way worked already... Looks cleaner to me (but this is just my opinion).

@sushantdhiman Perhaps we could just deprecate getterMethods in favor of virtual fields? What do you think?

[jayarjo]

I also prefer VIRTUAL fields, feels cleaner. You should definitely consider deprecating one or the other.

[gabegorelick]

It would be nice if this was documented in https://github.com/sequelize/sequelize/blob/master/docs/upgrade-to-v5.md, especially if a fix may take a while.

[cperryk]

This is also happening with instance.get({ plain: true }) — the resulting object does not include the result of any getters defined via the model's getterMethods

[hverlin]

This should definitely be mentioned in the breaking changes. Happy to update the documentation if that's ok?

[redevill]

When the getters ARE invoked, and they return undefined, it appears that the toJSON() falls on its face!

[github-actions]

This issue has been automatically marked as stale because it has been open for 14 days without activity. It will be closed if no further activity occurs within the next 14 days. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

[W0mpRat]

bump

[ephys]

getterMethods and setterMethods were deprecated in v6 and have been removed in v7: https://sequelize.org/docs/v7/other-topics/upgrade/#removal-of-previously-deprecated-apis