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

feat(NODE-3729): add withId to default return type for collection.find and collection.findOne #3039

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

PaulEndri
Copy link
Contributor

@PaulEndri PaulEndri commented Nov 16, 2021

Description

What is changing?

Adding a WithId wrapper to the return type of collection.findOne and collection.find

Is there new documentation needed for these changes?

No, should be self documented by ts output and is otherwise (probably) expected behavior

What is the motivation for this change?

Ran into a scenario similar to NODE-3729 and after seeing PR 3020 I wanted to suggest a more targeted solution that only impacts the find methods directly.

By changing only the return type of .find(); the _id property is correctly returned on all find operations. I limited the change to only instances where the return type for .find() isn't overridden because at that point the user is stating they want manual control over the resulting type and are responsible for their own _id definition (if one exists).

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>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@dariakp dariakp added External Submission PR submitted from outside the team Team Review Needs review from team tracked-in-jira Ticket filed in MongoDB's Jira system labels Nov 16, 2021
@dariakp dariakp changed the title types(NODE-3729): add withId to collection.find and collection.findOne feat(NODE-3729): add withId to default return type for collection.find and collection.findOne Nov 16, 2021
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@dariakp dariakp merged commit 52520aa into mongodb:main Nov 16, 2021
Comment on lines +733 to +735
find(filter: Filter<WithId<TSchema>>, options?: FindOptions): FindCursor<WithId<TSchema>>;
find<T>(filter: Filter<WithId<TSchema>>, options?: FindOptions): FindCursor<T>;
find(filter?: Filter<WithId<TSchema>>, options?: FindOptions): FindCursor<WithId<TSchema>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

@PaulEndri I think adding WithId to the Filter type here was a mistake. The other Filter types in this file are not using it.

avaly added a commit to plexinc/papr that referenced this pull request Dec 1, 2021
The following change in `mongodb` requires changes in the return types of
`find*` methods to include `WithId`:

- mongodb/node-mongodb-native#3039
@avaly avaly mentioned this pull request Dec 1, 2021
2 tasks
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 tracked-in-jira Ticket filed in MongoDB's Jira system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants