Update to Mongoose 5.0

[ttsirkia]

While gaining momentum again, updating dependencies is an important task. One of the key libraries is Mongoose. It should be checked how big changes updating to Mongoose 5.0 would mean for this project.

https://github.com/Automattic/mongoose/blob/master/History.md#500-rc0--2017-12-28

[htor]

or you can use your own mongoose instance with keystone.set('mongoose', require('mongoose')). at least until the dep is updated in keystone.

[ttsirkia]

Of course there are many ways but I'm more worried about what may break down because of the major version upgrade.

[stennie]

Looks like quite a few breaking changes that would likely require application updates as well.

Mongoose migration guide: https://github.com/Automattic/mongoose/blob/master/migrating_to_5.md

[stennie]

FYI, I tested updating to the latest Mongoose 4.x (currently 4.13.14) and this appears to be fine so far: PR #4701.

[ttsirkia]

It would be somehow natural to update to Mongoose 5 at this moment as there will be anyhow a major Mongoose update from 3 to 4. If it is updated directly to 5.0 before releasing the final Keystone 4.0, there won't be need to make this kind of major Mongoose update later.

[stennie]

Releasing a Keystone 4.0 GA is more important than upgrading to Mongoose 5, so this can wait until post 4.0.0 GA. There should also be a proper review of Mongoose 5 features/changes that Keystone might be able to take advantage of (which is different from a simple compatibility upgrade).

Since we've finally started a release candidate cycle, any remaining Keystone 4.0.0 changes will be critical fixes & polish. This will also give users of earlier versions of Keystone a chance to settle into Keystone 4 and any Mongoose 3 => 4 upgrades required.

Regards, Stennie

[ttsirkia]

That's also a good point to find out what new features Mongoose 5 could provide for the project.

[gautamsi]

I tried updating to mongoose 5.2.2, all tests passed except these two. after debugging it goes down to setMarkdown function inside MarkdownType.addToSchema method.

• [ ] FieldType: Markdown: Filter match should find empty and null string matches:
• [ ] FieldType: Markdown: Filter match should invert empty and null string matches:

Edit: further digging down reveals that it fails due to setter context being the Query object and not the Model object.

[vkarpov15]

Hi everyone, mongoose maintainer here. Anything I can do to help with this? I'm happy to debug any test failures if no one's on it yet.

[gautamsi]

I am not good at debugging mocha, but I can explain what happens in my test.

setup:

• clone from my branch https://github.com/gautamsi/keystone/tree/mongoose/upgrade_v5
• install dependency
• run local instance of mongodb, it drops existing database named 'test' during pretest
• run npm test

above two test would fail. I found in some attempt that the this context of setter setMarkdown in MarkdownType is of type Query instead of model. the line 66 fails at this.get, the error is thrown and the test never completes in timeout period.

I am kind of not able to go further.

[gautamsi]

I am able to fix now, also fixed deprecation notice for useNewUrlParser and collection.count.

collection.count is available from mongo driver 3.1 which is updated in mongoose 5.2 dependency. if we plan to use earlier version of mongoose 5 this may change, sending pull request for mongoose 5.2.2 update

edit: added mongoose 5.0 compatibility with this commit d8613c8a62a3eea96b1ed3b50de192500e61354f (not in pull request yet)

more info on earlier glitch with MarkdownType - http://thecodebarbarian.com/mongoose-4.10-runSettersOnQuery-option

[ttsirkia]

It is this line https://github.com/keystonejs/keystone/blob/master/index.js#L109 which causes the problems with pluralization. When toCollectionNamefunction is called, Mongoose doesn't use legacyPluralize. It can be passed as the second parameter to avoid the problem.

[justsilencia]

Any news on potentially updating keystone with mongoose 5.x?? I'm In the midst of building and app, and just realized I don't have .aggregate $lookup capability to create joins :( Also, I can't use

keystone.set('mongoose', require('mongoose'));

because it complete breaks the keystone sign in functionality. Any updates on this much appreciated, as I've had to make a rather ugly workaround for this...

[aleygues]

Up, it would be very useful.

[justsilencia]

Just wanted to update a workaround for for mongoose 4.x and lower. I figured out a way to gain aggregate functionality utilizing the cursor function like so:

keystone.list('Model').model.aggregate(pipeline)
        .cursor().exec()
        .toArray(function(err, data) {
            if (!err) {
                res.json(data);
            } else {
                console.log('Error  -->' + err);
            }
            
        });