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.

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

BREAKING CHANGE: use MongoDB node driver 6, drop support for rawResult option and findOneAndRemove() #13753

Merged
merged 12 commits into from Sep 15, 2023

Conversation

vkarpov15
Copy link
Collaborator

Summary

Use the latest commits to MongoDB Node driver's 6.0 branch to make sure Mongoose is compatible. 3 big changes:

  1. No more rawResult option. This PR replaces rawResult with the includeResultMetadata option
  2. Because of includeResultMetadata, we need to slightly restructure how orFail() works on findOneAndX(). Specifically, orFail() will now only error out when findOneAndX() doesn't return a document; previously, orFail() would also throw an error if upsert was set and a new document was upserted. With includeResultMetadata, detecting whether a document was upserted or not is no longer possible in general. However, I think this new approach makes more sense, or at least the TypeScript types make more sense, because TS types treat orFail() as making the query result non-nullable.
  3. No more findOneAndRemove(). findOneAndRemove() was just a legacy wrapper around findOneAndDelete(), didn't even have its own middleware.

Examples

@hasezoey
Copy link
Collaborator

git@github.com:mongodb/node-mongodb-native.git#6.0

i think this PR should be marked as Draft(WIP), or is this intended to be merged like this into 8.0 until it comes out?

Copy link
Collaborator

@AbdelrahmanHafez AbdelrahmanHafez left a comment

Choose a reason for hiding this comment

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

LGTM 👍

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.

mongodb 6.0.0 requires nodejs 16.20.1 or higher

@hasezoey
Copy link
Collaborator

also from bson release notes:

Strings of length 12 can no longer make an ObjectId

tests:

     strings of length 12 are valid oids (gh-3365):
    custom casters:
CastError: Cast to ObjectId failed for value "12charstring" (type string) at path "myId" because of "BSONError"
  with inheritance:

Actual message: "Cast to ObjectId failed for value "12charstring" (type string) at path "test" because of "BSONError""

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.

some minor markdown things

docs/migrating_to_8.md Outdated Show resolved Hide resolved
docs/migrating_to_8.md Outdated Show resolved Hide resolved
docs/migrating_to_8.md Outdated Show resolved Hide resolved
vkarpov15 and others added 2 commits September 1, 2023 14:33
Co-authored-by: hasezoey <hasezoey@gmail.com>
@vkarpov15
Copy link
Collaborator Author

@hasezoey fixed, can you please re-review?

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.

looks good to me now.

i dont know if it is related to this PR, but some replica set tests are timing out, and one test somehow tried to run while drop is occuring

test/model.test.js Show resolved Hide resolved
@vkarpov15
Copy link
Collaborator Author

@hasezoey I took a closer look and the test failures were indicative of a bug where we weren't passing session option to Document.prototype.deleteOne correctly, that's fixed now 👍

@vkarpov15 vkarpov15 merged commit 31517c7 into 8.0 Sep 15, 2023
38 of 39 checks passed
@vkarpov15 vkarpov15 added this to the 8.0 milestone Sep 15, 2023
@hasezoey hasezoey deleted the vkarpov15/mongodb-node-driver-6 branch September 15, 2023 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants