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

fix(NODE-6690): Remove extraneous Document in replaceOne return type #4383

Merged
merged 3 commits into from
Feb 5, 2025

Conversation

arturmuller
Copy link
Contributor

@arturmuller arturmuller commented Jan 28, 2025

Description

What is changing?

Type signature of replaceOne method no longer includes the general Document type. There doesn't seem to be any reason for the Document type to be included, as:

  1. returning a document from the replaceOne method is not documented anywhere
  2. it is not consistent with other methods
  3. based on Git history seems like an accident

Note: If the method indeed can return Document in some circumstances, perhaps it would be worth documenting in which situations that can happen, and perhaps type it with TSchema to follow the stronger typing of other methods like find and findOne.

Is there new documentation needed for these changes?

No. The docs already match my proposed change.

What is the motivation for this change?

More accurate typing is always helpful! Currently, TS is complaining about the return type when replaceOne is used in places which expect UpdateResult<TSchema> because of the extraneous Document, which makes using it awkward.

Release Highlight

Remove extraneous Promise<Document> in Collection.replaceOne return type

The return type signature of the replaceOne method no longer includes the general Promise<Document> type. Thanks to @arturmuller, the replaceOne type signature is now more accurate! 🎉

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Wasn't able to create a ticket. Not sure what I am doing wrong 😅
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@dariakp dariakp changed the title fix: Remove extraneous Document in replaceOne return type fix(NODE-6690): Remove extraneous Document in replaceOne return type Jan 28, 2025
@dariakp dariakp added tracked-in-jira Ticket filed in MongoDB's Jira system External Submission PR submitted from outside the team labels Jan 28, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@aditi-khare-mongoDB aditi-khare-mongoDB requested a review from a team as a code owner February 4, 2025 20:38
@dariakp dariakp added Team Review Needs review from team and removed tracked-in-jira Ticket filed in MongoDB's Jira system labels Feb 4, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@aditi-khare-mongoDB
Copy link
Contributor

Thanks for your submission @arturmuller!

You’re right, the replaceOne method's signature was imprecise and accurate typing is always important! Thanks again for catching this and submitting the improvement.

Copy link
Contributor

@aditi-khare-mongoDB aditi-khare-mongoDB left a comment

Choose a reason for hiding this comment

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

I'll merge this once the tests finish running!

@aditi-khare-mongoDB aditi-khare-mongoDB merged commit 6c81d4e into mongodb:main Feb 5, 2025
21 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Submission PR submitted from outside the team Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants