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

[vkarpov15]

Summary

Removes support for calling const q = Model.find(); await Model.find(q); - this syntax adds a lot of overrides in models.d.ts and queries.d.ts and these overrides add to a meaningful number of instantiations in TypeScript. The models.d.ts overrides alone add about 13k instantiations.

I made Model.find(q) throw an error, but Model.find().merge(q) is still supported - that is a better way to more explicitly merge one query into another.

Examples

[Copilot]

🟡 Changes recommended

The new contract should be locked in with additional/updated tests (notably TS negative type assertions and runtime coverage beyond find()/findOne()) to prevent regressions and fully match the broadened behavior implied by the shared canMerge() change.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Removes implicit “query-as-filter” merging (e.g. Model.find(existingQuery)) as a breaking change, to reduce TypeScript type instantiations and make query merging explicit via Query#merge().

Changes:

• Runtime: disallow merging when a Query instance is passed as the filter argument (now throws), while keeping Query#merge() as the supported mechanism.
• Types: remove Query<any, any> overloads from model and query method signatures to match the new runtime behavior and reduce TS instantiations.
• Docs/tests: add a migration note and a runtime regression test; remove the prior TS “should compile” snippet.

File summaries

File	Description
types/query.d.ts	Removes query-as-filter overloads from Query methods (compile-time enforcement).
types/models.d.ts	Removes query-as-filter overloads from Model methods (compile-time enforcement).
test/types/queries.test.ts	Removes the TS compile check that previously asserted query-as-filter support.
test/query.test.js	Adds a runtime test asserting find(query) / findOne(query) now reject and merge() still works.
lib/query.js	Updates canMerge() to reject Query instances as filter inputs.
docs/migrating_to_10.md	Documents the breaking change and the merge() migration path.

Review details

• Files reviewed: 4/6 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[AbdelrahmanHafez]

Didn't know that this was a thing. Glad it's gone.

Also, feels good:

[vkarpov15]

@AbdelrahmanHafez could you please take another look? I made some substantial changes since you last reviewed - I consolidated more overrides leading to about 8% fewer instantiations. Huge TypeScript perf win.

[AbdelrahmanHafez]

about 8% fewer instantiations. Huge TypeScript perf win

Nice!

I used Astra to take a look on the PR now, found a couple of findings that I think, at least the first, is worth fixing and adding regressions for:

---AI generated 👇---

1. Explicit result types lose option-based return types

Could we preserve the option-based return type when callers provide an explicit result type?

const result = await User.findOneAndUpdate<UserDoc>(
  { name: 'Alice' },
  { name: 'Bob' },
  { includeResultMetadata: true }
);

result.value;

Before this change, TypeScript returns ModifyResult<UserDoc>. After this change, it returns UserDoc | null, so accessing result.value fails type checking.

When we supply UserDoc, TypeScript uses the default type for Options instead of inferring it from the argument. The return type therefore loses includeResultMetadata: true.

The same issue makes explicit result types nullable with { upsert: true, new: true }.

2. Delete results become non-null with unsupported upsert options

Could we keep the upsert-specific return type limited to update and replace operations?

const result = await User.findOneAndDelete(
  { name: 'Alice' },
  { upsert: true, new: true }
);

result.name; // TypeScript accepts this, but result can be null.

findOneAndDelete() does not support upsert. However, its shared options type accepts these properties, and the new result helper uses them to remove null.

The operation still returns null when nothing matches. Both model and chained findByIdAndDelete() have the same issue.

This is a narrower concern because it requires options that do not apply to deletion.

[vkarpov15]

1. Explicit result types lose option-based return types

This is a good call and a blocking issue. I will undo the Options generic refactor because there's no good way to work around explicit result types losing option-based return types. Once the first generic parameter is specified, subsequent generics get their default value instead of inferring - just how TypeScript works.

2. Delete results become non-null with unsupported upsert options

This is no longer an issue if we revert the Options generic as per (1). Another good reason to revert the Options refactor because yes upsert should not remove null from the result type for findOneAndDelete... but on the other hand, findOneAndDelete(filter, options).findOneAndUpdate() should not have null in its result type.

Without the Options refactor, the instantiations reduction is closer to 4%: less good, but better than nothing.

[vkarpov15]

@AbdelrahmanHafez I found a better way to reduce instantiations - refactor out the document and query functions on Model into separate interfaces. Reduces instantiations from 395k to 360k while retaining the exact same overrides 🚀