Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BREAKING CHANGE: use mongodb@5, bson@5 #12955

Merged
merged 65 commits into from Feb 20, 2023
Merged

BREAKING CHANGE: use mongodb@5, bson@5 #12955

merged 65 commits into from Feb 20, 2023

Conversation

vkarpov15
Copy link
Collaborator

Summary

Couple of notes: mongoose.Types.ObjectId() no longer works, need to do new mongoose.Types.ObjectId(). ObjectId class's _bsontype is now 'ObjectId', not 'ObjectID'.

MongoClient.connect() no longer supports callbacks. We will likely need to drop callback support as well in this version.

Examples

@vkarpov15 vkarpov15 marked this pull request as draft January 26, 2023 22:19
hasezoey
hasezoey reviewed Jan 26, 2023
View reviewed changes
Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from what i can tell, this PR is not final yet, is there a ETA for mongodb's 5.0 release?

things to note:

  • debug console.log's present
  • documentation not yet noting that new Types.ObjectId() is required now
  • something seems like it does not work (see tests)?
  • usage of alpha dependencies (mongodb, bson)

@hasezoey hasezoey added this to the 7.0 milestone Jan 26, 2023
@vkarpov15
Copy link
Collaborator Author

@hasezoey you can ignore this PR for now, it is a very rough draft and not close to ready for merge. We're not shipping this with alpha dependencies.

No formal ETA for mongodb@5 release, but I would expect to see that released within the next few weeks.

@vkarpov15
fix most aggregation and connection tests for mongodb@5
9b51f98
@vkarpov15
fix: some more fixes for mongodb@5 compat
b65b97a
@vkarpov15
BREAKING CHANGE: make createCollection() an async function to fix som…
9b5663f 
…e test failures with mongodb@5
@vkarpov15
fix connection test failures with mongodb@5
7fe3c41
@vkarpov15
BREAKING CHANGE: remove Document.prototype.remove and Model.remove be…
15cf550 
…cause those are no longer in mongodb@5
@vkarpov15
BREAKING CHANGE: remove Model.mapReduce(), no longer supported in mon…
a4eb9c9 
…godb@5
@vkarpov15
make bulkWrite use promises and remove removeOptions
ffb1ef9
@vkarpov15
fix all but 1 querycursor test with mongodb@5
d6be7e9
@vkarpov15
fix last querycursor test with mongodb@5
34d8cf6
@vkarpov15
BREAKING CHANGE: drop support for overwrite option to queries, make d…
3174e29 
…ropDatabase() an async function (no callback), other mongodb@5 compat
@vkarpov15
style: fix lint
6c06bf9
@vkarpov15
fix: correct ObjectID -> ObjectId _bsontype in mongodb@5
0218623
@vkarpov15
clean up almost all remaining test failures, still have lsid error an…
681dccc 
…d some deleteOne hooks errors
baileympearson
baileympearson reviewed Jan 30, 2023
View reviewed changes
lib/connection.js Outdated Show resolved Hide resolved
@vkarpov15 vkarpov15 marked this pull request as ready for review February 12, 2023 22:14
@vkarpov15
Copy link
Collaborator Author

@hasezoey @AbdelrahmanHafez can you please take a look at this?

hasezoey
hasezoey requested changes Feb 14, 2023
View reviewed changes
Copy link
Collaborator

@hasezoey hasezoey left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a bunch of changes, the tests look way better now than the old nesting.

  • it seems like the benchmark for update was deleted instead of updated, should there be a new one?
  • typescript types seem to not have been updated yet
  • there are conflicts with the base branch

i have mainly some questions & style changes.

docs/migrating_to_7.md Outdated Show resolved Hide resolved
docs/migrating_to_7.md Show resolved Hide resolved
docs/migrating_to_7.md Outdated Show resolved Hide resolved
docs/migrating_to_7.md Show resolved Hide resolved
docs/migrating_to_7.md Outdated Show resolved Hide resolved
lib/helpers/query/validOps.js Show resolved Hide resolved
test/docs/validation.test.js Outdated Show resolved Hide resolved
lib/drivers/node-mongodb-native/connection.js Show resolved Hide resolved
test/model.populate.test.js Show resolved Hide resolved
test/model.test.js Show resolved Hide resolved
vkarpov15 and others added 6 commits February 15, 2023 11:12
@vkarpov15 @hasezoey
Update docs/migrating_to_7.md
12ec07a 
Co-authored-by: hasezoey <hasezoey@gmail.com>
@vkarpov15
docs: add error handling example for dropping callbacks
1ad9472
@vkarpov15 @hasezoey
Update docs/migrating_to_7.md
f587fb0 
Co-authored-by: hasezoey <hasezoey@gmail.com>
@vkarpov15 @hasezoey
Update docs/migrating_to_7.md
5f37eca 
Co-authored-by: hasezoey <hasezoey@gmail.com>
@vkarpov15 @hasezoey
Update docs/migrating_to_7.md
5f95588 
Co-authored-by: hasezoey <hasezoey@gmail.com>
@vkarpov15
address code review comments
37d85b8
IslandRhythms
IslandRhythms approved these changes Feb 15, 2023
View reviewed changes
Copy link
Collaborator

@IslandRhythms IslandRhythms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

hasezoey
hasezoey approved these changes Feb 15, 2023
View reviewed changes
Copy link
Collaborator

@hasezoey hasezoey left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

though noticed some missing Query.prototype functions in the callbacks removed section

docs/migrating_to_7.md Show resolved Hide resolved
@vkarpov15 vkarpov15 mentioned this pull request Feb 15, 2023
Merged
hasezoey
hasezoey reviewed Feb 17, 2023
View reviewed changes
lib/model.js Outdated Show resolved Hide resolved
test/query.test.js Outdated Show resolved Hide resolved
@hasezoey hasezoey mentioned this pull request Feb 18, 2023
Merged
@hasezoey
Copy link
Collaborator

should this PR also update mquery, or should this be done after this PR?

see mongoosejs/mquery#137

@vkarpov15
Merge pull request #13039 from Automattic/vkarpov15/drop-callbacks-ts…
8c3ceca 
…-changes

BREAKING CHANGE: remove callbacks from types, add notes about HydratedDocument changes
@vkarpov15
Copy link
Collaborator Author

@hasezoey we'll merge this PR into 7.0. Let's open a separate PR to bump mquery.

@vkarpov15 vkarpov15 merged commit f0f6c9e into 7.0 Feb 20, 2023
@hasezoey hasezoey deleted the vkarpov15/use-mongodb-node-driver-5 branch February 21, 2023 11:36
@hasezoey hasezoey mentioned this pull request Feb 23, 2023
Closed
@hasezoey hasezoey mentioned this pull request Mar 3, 2023
Merged
vkarpov15 added a commit to mongoosejs/in-memory-driver that referenced this pull request Jun 5, 2023
@vkarpov15
Merge pull request #6 from mongoosejs/vkarpov15/mongoose-7-support
4e2cdb2 
support Mongoose 7, re: Automattic/mongoose#12955
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imaksp imaksp imaksp left review comments

@hasezoey hasezoey hasezoey approved these changes

@baileympearson baileympearson baileympearson left review comments

@IslandRhythms IslandRhythms IslandRhythms approved these changes

@Uzlopak Uzlopak Awaiting requested review from Uzlopak

@AbdelrahmanHafez AbdelrahmanHafez Awaiting requested review from AbdelrahmanHafez

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
7.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@vkarpov15 @sapter @hasezoey @imaksp @baileympearson @IslandRhythms