-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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 TypeScript Change in updateMany from v8.5.5 to v8.6.0 #14862
Comments
This seems like expected behavior - await GuardianModel.updateMany(
{ $pull: { wards: { $in: [userId] } } }, // Filter with $pull operator
{},
{}
); Even worse, with the await GuardianModel.updateMany(
{ $pull: { wards: { $in: [userId] } } },
{ session },
); So I think this is a case where the new TypeScript code is doing its job, catching code that is dangerously wrong. |
Thank you for the clarification! I understand that $pull and other update operators are not valid in the filter parameter and that TypeScript is correctly catching this mistake. My intention was to create a reusable generic function to update documents across different models, but I realize now that I misunderstood the proper use of filter and update operators in this context. The stricter type definitions do help prevent mistakes like this. I agree that it is beneficial to have TypeScript catch these kinds of errors early on, even if it does make the transition from older versions a bit challenging. However, I do think it would be helpful to include a note in the documentation or release notes explaining this change and why it's now enforced. This would help developers understand the reasoning behind the stricter type checking, especially if they are upgrading from an older version where such issues might not have been caught by TypeScript. Thanks again for your response, and I appreciate the work being done to make Mongoose safer and more robust! |
Where's a good place to add a note that you would have seen? We do have a note in the changelog from #14764, do you have any suggestions for other places to highlight this change? |
Thank you for considering my feedback on where to add a note about the type changes! Suggestions for Improving Documentation To help developers notice this change, here are a few suggestions:
Regarding the Breaking Change I understand that this change improves type safety and helps catch errors early, which is beneficial. However, I still believe that this qualifies as a breaking change because it requires developers to update their existing code to adhere to the new types. In my case, the stricter types caused TypeScript errors in code that was previously valid, and the CI/CD pipeline began rejecting pull requests until the code was updated. This behavior fits the definition of a breaking change, as it breaks backward compatibility and necessitates modifications to existing codebases. A more gradual approach to introducing this change could have been:
Something like:
This approach would have balanced the benefits of improved type safety with a smoother developer experience. Thanks again for your consideration and for the great work on Mongoose! |
Prerequisites
Mongoose version
8.6.0
Node.js version
20.15.1
MongoDB server version
7
Typescript version (if applicable)
5.5.4
Description
Hi Mongoose team,
In version 8.6.0, the type definition for the filter parameter in updateMany was changed from FilterQuery to RootFilterQuery. This change restricts the use of certain MongoDB operators (such as $pull) that were allowed in 8.5.5.
This change causes TypeScript errors in existing codebases that rely on the old type definition, making it a breaking change. According to Semantic Versioning, this should have warranted a major version bump rather than a minor version change.
I think this change is a breaking change for existing codebases that use MongoDB operators in the filter parameter of updateMany and suggest that it may require a major version bump according to SemVer principles.
This is my prod code snippet, which works with version 8.5.5:
with version 8.6.0, I need to add something to replace the filter to keep Typescript happy:
This workaround does feel like a hack and is not intuitive, especially when your goal is to search all documents or apply a filter based on nested values.
Suggested Actions:
Revert the type change or consider bumping to a new major version.
Clearly document this change in the release notes to warn users.
Steps to Reproduce
Expected Behavior
The TypeScript code should compile without errors as it did in version 8.5.5, allowing MongoDB update operators like $pull in the filter parameter of updateMany.
Actual Behavior: The TypeScript compiler throws an error in version 8.6.0 due to the stricter type definition RootFilterQuery.
The text was updated successfully, but these errors were encountered: