Upgrade to Sequelize 6 #787
Conversation
|
Currently targeting the 6.0.0 beta release as that's what's available. |
The breakage comes from following Sequelize's lead and requiring a minimum node version of 10 and switching out Bluebird for native promises.
There was a problem hiding this comment.
Hey @kf6kjg thanks for contributing :) I really appreciate your help.
| @@ -26,7 +26,7 @@ export class BelongsToManyAssociation extends BaseAssociation { | |||
| const associatedClass = this.getAssociatedClass(); | |||
| const throughOptions = this.getThroughOptions(sequelize); | |||
|
|
|||
| const throughModel = typeof throughOptions === 'object' ? throughOptions.model : undefined; | |||
| const throughModel = typeof throughOptions === 'object' && typeof throughOptions.model !== "string" ? throughOptions.model : undefined; | |||
There was a problem hiding this comment.
Can you explain why this change is necessary?
There was a problem hiding this comment.
Because throughOptions.model has type string | typeof Model. The subsequent code isn't designed to handle the string case. Since my goals were to start with the minimal set of changes to make the code compatible with Sequelize 6 beta.6, this was the smallest fix to make the compiler and test suite happy.
Was unneeded code from a failed attempt that wasn't stripped before making the previous commit.
|
@RobinBuschmann Did I fully answer your questions and issues? |
|
Hope you are all well! This would be amazing to be released, as I think a lot of people that can benefit from it. |
|
Yesterday was the release of 6.1.0. It is worth checking for compatibility. |
|
At the moment I’m in crunch time through the end of the month. Is anyone else here able to see if this change works as is or not? |
|
I see this PR is open for quite some time now and still reference Sequelize 6 beta, which is not beta anymore and this package is not currently working with Sequelize 6. Is there any plan to fix this? Or is it supposed to work with Sequelize 6? |
|
As the author of this PR, yes the plan is to validate that it still works. But as I mentioned above I’m just a touch short on time at the moment. Would you be willing to build it and find out? It’d be very appreciated. :) |
|
Hey @kf6kjg, I tried to test the code from your PR but it doesn't seem to work. |
|
I was just about to exclude typescript-sequelize from my project to migrate to seqeulize@6 and then came across to this pull request. I'm going to test PR and see if it works for myself. |
|
and unfortunately, it didn't work for me. I gonna try to implement this on my own. |
|
@Awais000 i think a lot of people could benefit from this. if you do find a solution that would be dope. |
|
@brianschardt I started working on it. I go release by release to implement and accommodate all the breaking changes. Almost done with 6.0.0 because of depreciation of blueBird some of the tests got disturbed. |
|
#807 there you go @brianschardt @RobinBuschmann I opened a pull request. Please do take a look at it. |
|
How is this going ? :) |
|
is there any work in progress? |
|
I'm closing this as pretty much everything I did was done in the already merged #807. |
BREAKING CHANGE!: The breakage comes from following Sequelize's lead and requiring a minimum node version of 10 and switching out Bluebird for native promises.