-
-
Couldn't load subscription status.
- Fork 597
feat: MongoDB CommandOperationOptions #6792
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
base: master
Are you sure you want to change the base?
feat: MongoDB CommandOperationOptions #6792
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would very much like to see this done differently, isolated inside the @mikro-orm/mongodb package instead of leaking on the core level (IDatabaseDriver.ts)
we can reexport necessary interfacing from the driver package to alter them if there isnt a better way
(also some tests are needed)
|
|
||
| async deleteMany<T extends object>(collection: string, where: FilterQuery<T>, ctx?: Transaction<ClientSession>): Promise<QueryResult<T>> { | ||
| return this.runQuery<T>('deleteMany', collection, undefined, where, ctx); | ||
| async deleteMany<T extends object>(collection: string, where: FilterQuery<T>, deleteOptions: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a breaking change on API level, we cant just change shape of existing parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for reviewing the request and for the quick feedback 🙌
You're absolutely right — changing the method signature of deleteMany is a breaking change at the API level.
When exploring the codebase, I noticed that deleteMany on the connection level is currently only used internally by MongoDriver, so I went with updating the method signature to support passing CommandOperationOptions. But you're correct — it’s better to avoid such changes unless strictly necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont mind changing the parameter now, but it needs to support the old signature too, which could be tricky in this case (it already was an object). better just add a new parameter and clean this up in v7
| upsert?: boolean; | ||
| loggerContext?: LogContext; | ||
| /** MongoDb only @CommandOperationOptions */ | ||
| commandOperationOptions?: Record<string, any>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would rather not pollute the shared interfacing with mongo specific options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the thoughtful review and valuable feedback
You're absolutely right — avoiding Mongo-specific options in shared interfaces makes perfect sense, and I appreciate you pointing that out. I’ll revisit the problem and explore an alternative that respects the current abstractions.
If you have a preferred way of handling this kind of driver-specific option (like passing comment), I’d be grateful for your guidance.
Alternatively, would it make more sense to raise a feature request first so we can discuss the use case and potential solutions there?
Thanks again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we already have an issue for this, its basically the same as #1603, right?
IMO the best way would be to extend the interface from the core package with the new options, on the mongo package level, and reexport it with the same name. we do the same with the driver specific MikroORM and EntityManager classes
🛠 Changes
This PR adds support for passing native MongoDB
CommandOperationOptions(such ascomment,maxTimeMS, etc.) from the MikroORM driver layer all the way down to the native MongoDB client.Motivation
In our use case, we wanted to leverage MongoDB's native comment option to associate contextual metadata (such as audit information) with each write operation. However, MikroORM currently does not expose a way to pass these options through its driver abstraction.
What's changed?
A new optional field
commandOperationOptions?: Record<string, any>has been added toNativeInsertUpdateOptions<T>and related option interfaces.This parameter is propagated through:
MongoDriver.nativeInsert,nativeInsertMany,nativeUpdate,nativeUpdateMany,nativeDeleteMongoConnection.deleteManyMongoConnection.runQuery(), where it is spread into the final native client operation optionsThis makes it possible to pass custom options like:
🚧 Limitations
No new tests have been added yet, but the existing test suite is passing (yarn test).
This feature currently assumes that commandOperationOptions is a flat object — no validation or type safety for deeper command semantics has been implemented.
✅ Next Steps
Add dedicated test coverage for the propagation logic.
Optionally consider stricter typing by using
Partial<CommandOperationOptions>instead ofRecord<string, any>.