sequelize: Instance#toJSON doesn't apply included models customized toJSON

I think it might be a good idea to change the current Sequelize behavior to recursively respect custom toJSONs that could be defined around.

Sorry to bother you @mickhansen, do you remember why not to support this? Is there any downside?

In other words, why not change the current behavior, as shown by you:

But generally JSON.stringify will call toJSON, Sequelizes built-in toJSON will call get({plain: true}) which will recursively call get({plain: true}) on all included.

To recursively call toJSON instead of calling get({plain: true})?

Recursively calling toJSON would, in turn, automatically call get({plain: true}) anyway, unless a custom toJSON was defined there…

In my use-case I don’t just want to hide fields, I just want to use a custom toJSON() function that is used for included instances as well.

On Mon, Jun 15, 2020 at 10:15:25, Johan le Roux < notifications@github.com > wrote:

A simple solution to this problem that follows the structure of Sequelize, is to add either an array called hidden to the modelOptions section of the model definition or a boolean hidden field on the attribute definition on a model.

Then by updating the internal lib/model.js:get() function we can hide by default all fields which are defined as hidden, even when including the relationships of a model through get({ plain: true}). This will also work when calling toJSON as it calls get({ plain: true }) internally.

Model definition

module. exports = ( sequelize , DataTypes ) => { const user = sequelize. define ( ‘user’ , { email : DataTypes. STRING , password : DataTypes. STRING , } , { hidden : [ ‘password’ ] } ) return user }

Return model and hide password by default

const user = User. findOne ( ) return user. get ( )

Return model and show password

const user = User. findOne ( ) return user. get ( { hidden : false } )

Return password directly

const user = User. findOne ( ) return user. get ( ‘password’ )

I have played around with this solution locally and got a prototype working, think this will be a great non-breaking feature that can be included in the core

— You are receiving this because you commented. Reply to this email directly, view it on GitHub ( https://github.com/sequelize/sequelize/issues/3891#issuecomment-644008486 ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/ABUVAASFPZWGSNA7WVSMX3TRWXRC3ANCNFSM4BHK6AZA ).

@lukeberry99 I was able to solve this by adding a getter method to the field that returns undefined, like below:

firstName: {
      type: DataTypes.STRING,
      get() {
        return undefined;
      }
    }

See my answer on SO for more details - https://stackoverflow.com/a/59337434/1346528

Here is what i did to solve this issue:

We use a base model class. All of our models extend from this abstract class.

Inside, we override both toJSON and get methods.

The main idea is to check in the overriden get method if we are in a toJSON serialization context.

Notice the extra parameter that we add in the get options object.

Also we must delete the unwanted fields only when the get method is called for retrieving all fields, and not a unique field.

So we simply check the arguments, if a string property is provided, then we just return the original return value.

Otherwise, we check if we have an options objet, and if it contains our special “toJSON” option. If it is the case, then we can safely delete the removeFromJson fields.

import { Model } from 'sequelize'
import _ from 'lodash'

abstract class BaseModel extends Model {

    public abstract readonly removeFromJson: (keyof this)[]
    
    public readonly id!: string
    public readonly createdAt!: Date
    public readonly updatedAt!: Date

    public get(...args: any[]): unknown {
        
    	const o = super.get(...args)

    	if (args.length > 1 || typeof args[0] === 'string')
    		return o

    	const options = args[0]

    	if (options === undefined)
    		return o

    	if (options.toJSON === true)
    		for (const field of this.removeFromJson)
    			delete o[field]

    	return o
    }

    public toJSON(): Partial<this> {

    	const json = _.cloneDeep(this.get({ plain: true, toJSON: true })) as Partial<this>

    	return json
    }
}

We need to declare our non-serializable fields in our sub classes, like this:

import BaseModel from './BaseModel'

class MyModel extends BaseModel {

    public readonly removeFromJson: (keyof this)[] = [ 'secret' ]

    public readonly foo!: string
    public readonly bar! string
    public readonly secret!: string
}

Now the toJSON method will delete every field included in removeFromJson, and recursively in nested instances of other models (for as long as they extend the BaseModel class as well of course).

What do you think ?

Sure, you can still overwrite toJSON, but just remember it isn’t necessarily called if the child of another instance. In that case you’ll have to overwrite the parent toJSON aswell so it doesn’t call get like Sequelize does by default.