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

findOneAndUpdate typing does not include null result #10820

Closed
janeisenmenger opened this issue Sep 30, 2021 · 2 comments
Closed

findOneAndUpdate typing does not include null result #10820

janeisenmenger opened this issue Sep 30, 2021 · 2 comments
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@janeisenmenger
Copy link

This issue occurred when updating from 6.0.5 to 6.0.8

The behaviour before was that findOneAndUpdate would either return the new/old document (depending on the options) or null, when no document was updated.
With this commit the typing of the function was changed to not include null anymore.

However, it is still possible to receive a null value, regardless of the typing. Steps to reproduce:

  • call findOneAndUpdate on an empty repository (any data or model will do)
  • Check the return value -> This is were we receive null, even though it shouldn't be according to the typing.

My proposition would be to add the null value back to the typing or throw a matching error.

Versions used:

  • Node.js: 14.17.5
  • MongoDB: 4.2
  • Mongoose: 6.0.8
@IslandRhythms IslandRhythms added the needs clarification This issue doesn't have enough information to be actionable. Close after 14 days of inactivity label Sep 30, 2021
@IslandRhythms
Copy link
Collaborator

Where does it say on the doc pages that null is not a possible value?

@janeisenmenger
Copy link
Author

Hey @IslandRhythms!
The documentation doesn't mention null on the corresponding findOneAndUpdate page, or at least I couldn't find it.
Regardless of the documentation, I think the typing should reflect the return values the function could have, and currently it does not.
So: When looking at the typing of Model.findOneAndUpdate, one can see that before the update, the signature was as follows:
Query<T | null, EnforceDocument<T, TMethods>, T>. Note the explicit T | null as part of the Query return type.
Now, after the update, the same function on the Model has the signature QueryWithHelpers<EnforceDocument<T, TMethods, TVirtuals>, EnforceDocument<T, TMethods, TVirtuals>, TQueryHelpers, T> - Where null is not a possibility anymore, at least according to the types.

Note that the signature of Query has not changed in that regard, it still allows DocType | null - We simply restrict the type in the Model, which we probably shouldn't.

@IslandRhythms IslandRhythms added typescript Types or Types-test related issue / Pull Request and removed needs clarification This issue doesn't have enough information to be actionable. Close after 14 days of inactivity labels Oct 1, 2021
@vkarpov15 vkarpov15 added this to the 6.0.10 milestone Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

No branches or pull requests

3 participants