moving model.associate outside classMethods due to sequelize@4.0.0 by geluso · Pull Request #470 · sequelize/cli

Conversation

@geluso

I'm trying to figure out if sequelize-cli is supposed to match breaking changes introduced in sequelize@4.0.0. If yes, the model.associate method needs to be moved outside classMethods because classMethods was removed in sequelize@4.0.0.

Review my comment here: #468

@wayne-o

This desperately needs to be merged in :/

@sushantdhiman

This will be a breaking change, so can't release to v3

@wayne-o

I figured out the modification that needs to happen - all good in the interim :)

@sushantdhiman

I will post sample from v4 upgrade guide for others
http://docs.sequelizejs.com/manual/tutorial/upgrade-to-v4.html

Previous :

const Model = sequelize.define('Model', {
    ...
}, {
    classMethods: {
        associate: function (model) {...}
    },
    instanceMethods: {
        someMethod: function () { ...}
    }
});

New :

const Model = sequelize.define('Model', {
    ...
});

// Class Method
Model.associate = function (models) {
    ...associate the models
};

// Instance Method
Model.prototype.someMethod = function () {..}

@geluso

@sushantdhiman thank you for a direct clarifying response and clear progress forward!

@mandreko

I'm all for this change. I just spent an evening trying to figure out why my models and relationships were not working. I eventually compared the generated code from the cli to the example site to see the difference in the models.

@buddylindsey

@sushantdhiman What would it take to get this merged? If @geluso doesn't want to fix the merge conflict could I branch off of his and fix it and create a new PR? Or if @geluso does fix the merge conflict is that all it would take to merge?

Was wanting to see if I could help a bit move cli v4 along.

@sushantdhiman

@buddylindsey Please open a new branch and I will merge it, I am unable to find some time to complete #441 so let's fix incompatibilities first then think about other features

codetriage-readme-bot pushed a commit to codetriage-readme-bot/cli that referenced this pull request

Jun 5, 2019
Implement upgrade command

Labels