BREAKING CHANGE: remove automatic query merging in find() etc.

docs/migrating_to_10.md

@@ -28,3 +28,18 @@ Cast values explicitly instead of relying on adhoc type casting.
 
 The function signature for `Document#set()` and its alias `Document#$set()` is now `function set(path, val, options?)` - the 3rd argument is now `options`.
 The `type` argument has been removed.
+
+## Removed support for passing a query to model query methods
+
+Mongoose 10 no longer supports passing a query instance as the filter to model query methods like `find()` and `findOne()`.
+
+If you need to copy a query into another query, use `Query.prototype.merge()`:
+
+```javascript
+const query = User.find({ status: 'active' }).select('name');
+const queryToRun = User.find().merge(query);
+
+await queryToRun.exec();
+```
+
+`Query.prototype.merge()` copies the query's conditions, field selection, and options to the query it is called on.

lib/query.js

@@ -3007,6 +3007,10 @@ Query.prototype.countDocuments = function(conditions, options) {
 
   if (canMerge(conditions)) {
     this.merge(conditions);
+  } else if (conditions != null) {
+    this.error(
+      new ObjectParameterError(conditions, 'filter', 'countDocuments')
+    );
   }
 
   if (options != null) {
@@ -3661,6 +3665,10 @@ Query.prototype.findOneAndDelete = function(filter, options) {
 
   if (canMerge(filter)) {
     this.merge(filter);
+  } else if (filter != null) {
+    this.error(
+      new ObjectParameterError(filter, 'filter', 'findOneAndDelete')
+    );
   }
 
   options && this.setOptions(options);
@@ -4468,9 +4476,8 @@ function _update(query, op, filter, doc, options) {
   query.op = op;
   doc = doc || {};
 
-  if (!(filter instanceof Query) &&
-      filter != null &&
-      filter.toString() !== '[object Object]') {
+  if (filter instanceof Query ||
+      (filter != null && filter.toString() !== '[object Object]')) {
     query.error(new ObjectParameterError(filter, 'filter', op));
   } else {
     query.merge(filter);
@@ -5766,8 +5773,7 @@ Query.prototype.model;
  */
 
 function canMerge(value) {
-  return value instanceof Query || utils.isObject(value) || value === null;
-
+  return (!(value instanceof Query) && utils.isObject(value)) || value === null;
 }
 
 /*!

test/query.test.js

@@ -3000,6 +3000,45 @@ describe('Query', function() {
 
       assert.equal(res.owner.name, 'Val');
     });
+
+    it('does not merge queries passed as a filter to find() and findOne()', async function() {
+      const Test = db.model('Test', new Schema({ name: String }));
+
+      const q = Test.find({ name: 'foo' });
+
+      await assert.rejects(
+        Test.find(q).exec(),
+        /Parameter "filter" to find\(\) must be an object/
+      );
+      await assert.rejects(
+        Test.findOne(q).exec(),
+        /Parameter "filter" to findOne\(\) must be an object/
+      );
+      await assert.rejects(
+        Test.countDocuments(q).exec(),
+        /Parameter "filter" to countDocuments\(\) must be an object/
+      );
+      await assert.rejects(
+        Test.findOneAndDelete(q).exec(),
+        /Parameter "filter" to findOneAndDelete\(\) must be an object/
+      );
+      await assert.rejects(
+        Test.updateOne(q, { name: 'bar' }).exec(),
+        /Parameter "filter" to updateOne\(\) must be an object/
+      );
+      await assert.rejects(
+        Test.updateMany(q, { name: 'bar' }).exec(),
+        /Parameter "filter" to updateMany\(\) must be an object/
+      );
+      await assert.rejects(
+        Test.replaceOne(q, { name: 'bar' }).exec(),
+        /Parameter "filter" to replaceOne\(\) must be an object/
+      );
+
+      // `merge()` still supports queries
+      const res = await Test.find().merge(q);
+      assert.deepStrictEqual(res, []);
+    });
   });
 
   describe('Query#validate() (gh-7984)', function() {

test/types/models.test.ts

@@ -1,6 +1,5 @@
 import mongoose, {
   AggregateOptions,
-  CallbackError,
   DeleteResult,
   Document,
   HydratedDocument,
@@ -320,9 +319,6 @@ function find() {
   Project.find({});
   Project.find({ name: 'Hello' });
 
-  // just callback; this is no longer supported on .find()
-  Project.find((error: CallbackError, result: IProject[]) => console.log(error, result));
-
   // filter + projection
   Project.find({}, undefined);
   Project.find({}, null);
@@ -790,6 +786,13 @@ async function gh13705() {
 
   const findOneAndUpdateResWithMetadata = await TestModel.findOneAndUpdate({}, {}, { lean: true, includeResultMetadata: true });
   expect(findOneAndUpdateResWithMetadata).type.toBe<ModifyResult<{ name?: string | null | undefined }>>();
+
+  const findOneAndUpdateResWithMetadataAndOverride = await TestModel.findOneAndUpdate<{ answer: 42 }>(
+    {},
+    {},
+    { includeResultMetadata: true }
+  );
+  expect(findOneAndUpdateResWithMetadataAndOverride).type.toBe<ModifyResult<{ answer: 42 }>>();
 }
 
 async function gh16413() {

test/types/queries.test.ts

@@ -735,8 +735,8 @@ async function gh13142() {
       options: Options
     ): Promise<
         Options['lean'] extends true
-          ? Pick<Blog, Extract<keyof Projection, keyof Blog>> | null
-          : HydratedDocument<Pick<Blog, Extract<keyof Projection, keyof Blog>>> | null
+          ? mongoose.ApplyProjection<Blog, Projection> | null
+          : HydratedDocument<mongoose.ApplyProjection<Blog, Projection>> | null
     > {
       return this.blogModel.findOne(filter, projection, options);
     }
@@ -815,6 +815,29 @@ async function gh14190() {
   expect(res2).type.toBeAssignableTo<
     ModifyResult<ReturnType<(typeof UserModel)['hydrate']>>
   >();
+
+  const res3 = await UserModel.find().findOneAndUpdate(
+    { name: 'test' },
+    { name: 'updated' },
+    { includeResultMetadata: true }
+  );
+  expect(res3).type.toBeAssignableTo<
+    ModifyResult<ReturnType<(typeof UserModel)['hydrate']>>
+  >();
+
+  const upserted = await UserModel.find().findOneAndUpdate(
+    { name: 'test' },
+    { name: 'updated' },
+    { upsert: true, new: true }
+  );
+  expect(upserted).type.toBe<ReturnType<(typeof UserModel)['hydrate']>>();
+
+  const upsertedById = await UserModel.find().findByIdAndUpdate(
+    '0'.repeat(24),
+    { name: 'updated' },
+    { upsert: true, returnDocument: 'after' }
+  );
+  expect(upsertedById).type.toBe<ReturnType<(typeof UserModel)['hydrate']>>();
 }
 
 function mongooseQueryOptions() {
@@ -1061,11 +1084,6 @@ async function gh15779() {
   expect(v8Filter.age).type.toBeAssignableFrom(42);
   expect(v8Filter.age).type.not.toBeAssignableFrom('taco');
 
-  const TestModel = model('Test', new Schema({ age: Number, name: String }));
-  const query = TestModel.find({ age: { $gt: 18 } });
-  TestModel.find(query); // Should compile without errors
-  TestModel.findOne(query);
-  TestModel.deleteMany(query);
 }
 
 async function gh15786() {

types/callback.d.ts

@@ -1,8 +1,3 @@
 declare module 'mongoose' {
-  type CallbackError = NativeError | null;
-
-  type Callback<T = any> = (error: CallbackError, result: T) => void;
-
-  type CallbackWithoutResult = (error: CallbackError) => void;
-  type CallbackWithoutResultAndOptionalError = (error?: CallbackError) => void;
+  type CallbackWithoutResultAndOptionalError = (error?: NativeError | null) => void;
 }