TypeScript errors with .lean() after upgrading from Mongoose 8.7.1 to 8.7.2 #14974
Comments
|
EDIT: I've opened a separate issue to avoid polluting this one |
|
Seeing the same thing, probably related to #14902 and #14967 Here is a fully reproducible example which worked in 8.7.1: import { Schema, model } from 'mongoose';
import type { Document, ObjectId } from 'mongoose';
export interface Foo {
_id: ObjectId;
bar: string;
}
export type FooDocument = Foo & Document<ObjectId>;
const schema = new Schema<Foo>(
{
bar: { type: String, required: true },
},
{ collection: 'foo', timestamps: true },
);
export const FooModel = model<Foo>('foo', schema);
const test: FooDocument[] = await FooModel.find({ bar: 'asd' }).lean(); |
|
@vkarpov15 what do you think about this? to my knowledge |
|
Binary is slightly different from Buffer. The error message makes it seem like Mongoose is converting some internal type's Buffer properties into Binary when it shouldn't. I'm investigating, but haven't found an explanation yet. |
|
Ok I found what the issue is, your schema definition should be the following: import type { Document, Types } from 'mongoose';
export interface Foo {
_id: Types.ObjectId;
bar: string;
}
export type FooDocument = Foo & Document<Types.ObjectId>;
|
|
There doesn't seem to be a good way to make it so that export interface Foo {
_id: ObjectId;
bar: string;
}with: export interface Foo {
_id: Types.ObjectId;
bar: string;
}because |
|
I can say the issue is still present, when using lean our project typescript breaks, I don't think it's right that this issue was closed, the new version just breaks an entire project |
|
Hi @vkarpov15, Thank you for your time and effort in addressing this issue. I appreciate the attention given to the example provided by @richardsimko. However, I wanted to clarify that the core TypeScript bug I originally reported still persists and remains unaddressed. The main issue arises after upgrading from Mongoose 8.7.1 to 8.7.2, where TypeScript errors occur when using the .lean() method in queries that previously worked without any issues. This seems to be related to the changes in the type utility chain, specifically the swapping of FlattenMaps and BufferToBinary in mongoose/types/query.d.ts. I believe this Typescript but was introduced with changes merged a couple of weeks ago to try and fix #14902. Previously, the type chain was:
But in version 8.7.2, it has changed to:
This alteration appears to introduce stricter typing or structural mismatches when dealing with lean documents, especially without explicit typing in While the other user's example may have had typing issues, the fundamental problem is not isolated to that case. The issue impacts any code that utilizes Could we please revisit this matter? I believe the change introduced in version 8.7.2 has inadvertently caused a breaking change in the TypeScript typings. It would be greatly appreciated if the issue could remain open until a resolution is found. Thank you for your understanding and for your continued efforts to improve Mongoose. |
Thanks you for addressing this case in a very detailed answer, I apologize for my attitude earlier 😃 and of course very appreciate the Mongoose team work 🙏🏻 |
|
@vkarpov15 As others have pointed out this issue isn't actually solved by your recommended solution and should be reopened. Here is the same repro with your suggested change which still throws an error. import { Schema, model } from 'mongoose';
import type { Document, Types } from 'mongoose';
export interface Foo {
_id: Types.ObjectId;
bar: string;
}
export type FooDocument = Foo & Document<Types.ObjectId>;
const schema = new Schema<Foo>(
{
bar: { type: String, required: true },
},
{ collection: 'foo', timestamps: true },
);
export const FooModel = model<Foo>('foo', schema);
const test: FooDocument[] = await FooModel.find({ bar: 'asd' }).lean();
I'm again asking for a #14967 to be reverted until a stable solution has been found, as others have mentioned this is a breaking change that was released as a patch version and should be rolled back. |
|
(original issue description)
(later comment)
just wanna point out that #14967 completely added the new This is how the lean type was in tag 8.7.1: Lines 213 to 215 in 02c5efd
to my knowledge in mongoose any type change can be in any version, though it had mostly stabilized since 7.x (speaking from a typegoose upgrade perspective) i also wanna point out that the examples from this comment and this comment want to assign a lean type to a document type. A lean type does not have anything from type A fixed version could look like: /* eslint-disable @typescript-eslint/no-unused-vars */
// NodeJS: 22.8.0
// Typescript 5.3.3
import * as mongoose from 'mongoose'; // mongoose@8.8.0
interface Foo {
_id: mongoose.Types.ObjectId;
bar: string;
}
type FooDocument = Foo & mongoose.Document<mongoose.Types.ObjectId>;
type FooLean = mongoose.GetLeanResultType<Foo, Foo, unknown>;
const schema = new mongoose.Schema<Foo>(
{
bar: { type: String, required: true },
},
{ collection: 'foo', timestamps: true }
);
const FooModel = mongoose.model<Foo>('foo', schema);
async function main() {
const test1: FooLean[] = await FooModel.find({ bar: 'asd' }).lean();
const test2: Foo[] = await FooModel.find({ bar: 'asd' }).lean();
// const test3: FooDocument[] = await FooModel.find({ bar: 'asd' }).lean();
}
main();(Reproduction repository & branch) If the issue still persists when removing the |
|
Hi @hasezoey, Thank you for joining the conversation and for your valuable insights. You correctly pointed out that BufferToBinary was introduced in version 8.7.2. I apologise for any confusion regarding references to changes in the type utility chain; I was referencing the code I was testing while tracing the root cause of the typing issue. I believe we are now addressing two separate issues:
I still consider the recent changes in the TypeScript types to be a breaking change. The necessity to refactor existing code to comply with the new types suggests that the changes are not backward-compatible. While I acknowledge that these types of changes may offer improvements or are necessary corrections, the core concern is that such changes should not be introduced in a minor or patch release according to Semantic Versioning (SemVer) principles. Some might argue that types do not affect the runtime behaviour of JavaScript code and that changing types is merely a correction, especially if the previous types were incorrect or incomplete. However, TypeScript types are more than just annotations; they define the contract between the library and its consumers. When a package exposes types, those types become part of its public API. Altering them can significantly impact how consumers interact with the library, potentially causing type errors or necessitating code changes. Allowing non-backward-compatible type changes without a major version bump undermines the purpose of SemVer. Consumers rely on version numbers to gauge the impact of updates. Unexpected breaking changes can lead to integration issues and erode trust in the package's stability. Your code example revealed that my team will need to undertake significant refactoring of our model definitions to accommodate the use of plain JavaScript objects returned from using .lean(), while also adhering to the conventions introduced in Mongoose 7+. In our case, this involves refactoring numerous models and their respective usages. This underscores the impact of the typing changes and supports the view that they constitute a breaking change. I understand that maintaining detailed documentation for every type of change can be resource-intensive, especially in fast-moving projects. However, when types are integrated and maintained within the repository, they should be treated with the same level of importance as the code itself. Documentation should reflect type changes, providing guidance on how to adapt to new requirements. This transparency helps consumers smoothly transition between versions.
As mentioned above, your code example indicates that we need to refactor our models and their usage to comply with the typing conventions introduced in Mongoose 7+, where extending Document in TypeScript interfaces is discouraged. However, I am cautious about using GetLeanResultType during refactoring. The reason is that I could not find any mention of it in the Mongoose documentation. Unless one is a Mongoose expert, it is unlikely that developers are aware of this utility type. For example, searching the documentation for
My understanding is that after Mongoose 7, we need separate types or interfaces to work with .lean() results and hydrated Mongoose documents. To avoid using GetLeanResultType and manually extending Document like type FooDocument = Foo & mongoose.Document<mongoose.Types.ObjectId>;, we are planning to use an approach similar to the following: This approach allows us to work with both lean results and hydrated documents without relying on undocumented or potentially unstable types like GetLeanResultType. It also aligns with the practice of avoiding direct extension of Documents in our interfaces. A developer might also consider the user of Possible Temporary Workaround If reverting the type changes is not an option and the new types are to be kept, we are considering using a temporary workaround by directly typing .lean() where Foo still extends Document, even though we understand this is not ideal and undermines the benefits of type safety. This approach would allow us to keep our project moving forward while we allocate time for the necessary refactoring to properly handle .lean() results and hydrated documents. We suspect that other teams might resort to similar measures under time constraints. I appreciate your efforts in improving Mongoose and understand the challenges in balancing progress with stability. I hope that this feedback helps in considering the impact of typing changes on the developer community and prompts a discussion on how to better manage such changes in the future. |
I totally agree to that, but from my knowledge types in mongoose are "second class" and can be changed at any time, though from experience it has been more stable since 7.x (coming from a typegoose upgrade standpoint). For any more discussion on the general compatibility of types, please refer to #15017.
I just used @vkarpov15 what do you think about reverting the mention change? Guessing that now new minor version (from 8.7 to 8.8) has come out, i think it is unlikely to get a revert or a new 8.7 version with a revert. |
|
Thank you, @hasezoey, for your reply. The tests were also updated in the same PR to match the inclusion of the new type: 8825b76 @ test/types/schema.test.ts, and you were one of the reviewers/approvers. If the tests had not been updated along the type changes, the PR would not have passed the deployment tests, I guess, as I do not know about Mongoose's CI/CD implementation. We determined we will stick to version 8.7.1 while we fix the issues your code example and the type changes helped us uncover. |
Do you mean this change? as otherwise there are only new test cases. |
|
"what do you think about reverting the mention change?" I think we should not revert this change. The line between a bug fix and a breaking change can get blurry, especially when users rely on incorrect behavior. However, in this case, I think the TypeScript types are doing their job by highlighting code that is objectively incorrect. If you're expecting You can always work around this issue using |
|
@hasezoey, the modifications in schema.test.ts seem directly related to supporting the new type definitions. If those tests had not been updated, they likely would have failed, indicating a shift in how consumers should interact with Mongoose. This reinforces the idea that the changes are significant. @vkarpov15, while I understand that these updates may provide useful improvements, I hope that future changes, even if they enhance current functionality, will adhere to Semantic Versioning best practices. This will ensure a smoother experience for all users and maintain trust in the library's stability. If such changes continue to occur, we will likely keep facing issues like this. I reported something similar two months ago: #14863, and it seems I’m not alone, as @richardsimko mentioned. Thank you, guys, for the discussion! I learned a lot about Mongoose along the way. |
Prerequisites
Mongoose version
8.7.2
Node.js version
20.15.1
MongoDB server version
7.0.14
Typescript version (if applicable)
5.6.3
Description
Hi Mongoose team,
After upgrading Mongoose from version 8.7.1 to 8.7.2, I'm encountering TypeScript errors when running queries that use the .lean() method. The issue seems to be related to changes in the type utility chain, specifically in how FlattenMaps and BufferToBinary are applied.
Here’s the TypeScript error:
Code that previously ran without issues, such as:
or
is now throwing the above TypeScript error.
Previously, the type chain in mongoose\types\query.d.ts was:
Require_id<FlattenMaps<BufferToBinary<RawDocType>>>in the code:
But after the update, the order of FlattenMaps and BufferToBinary has been swapped to:
Require_id<BufferToBinary<FlattenMaps<RawDocType>>>like:
This change appears to have introduced stricter typing or structural mismatches in queries that return lean documents, particularly when using .lean() or .lean() without explicit typing. The flipping of FlattenMaps and BufferToBinary seems to cause issues when dealing with Map or Buffer fields in the schema. I think this is breaking change, and should be reverted.
Steps to Reproduce
or
This will run without issues.
Upgrade to Mongoose 8.7.2
Run the same queries, and then TypeScript will show an error.
Expected Behavior
Expected Behavior:
Queries using .lean() should return plain objects that conform to the expected TypeScript interface (e.g., Blog[] in the case of a Blog model) without any TypeScript errors, as it worked in version 8.7.1.
As a workaround, I’ve had to explicitly type the result of .lean(), but this was not required in previous versions:
The text was updated successfully, but these errors were encountered: