moving model.associate outside classMethods due to sequelize@4.0.0 by geluso · Pull Request #470 · sequelize/cli
Conversation
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
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 () {..}
@sushantdhiman thank you for a direct clarifying response and clear progress forward!
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.
@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.
@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, 2019This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters