Skip to content
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

types(models+query): infer return type from schema for 1-level deep nested paths #14632

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

vkarpov15
Copy link
Collaborator

Fix #14615
Re: #13836
Re: #12064

Summary

Fixes #14615 by inferring the type of 1-level deep schema paths to help with query filters. Consider the following schema:

  const FooSchema = new Schema({
    one: { type: String }
  });

  const MyRecordSchema = new Schema({
    _id: { type: String },
    foo: { type: FooSchema }
  });

Currently, Mongoose can infer await MyRecord.distinct('foo').exec() return type because foo is a top-level path. But not await MyRecord.distinct('foo.one').exec() because foo.one is nested.

As per discussion in #12064, we don't want to infer arbitrarily deep nested paths: too risky because we don't want to run into the dreaded Type instantiation is excessively deep and possibly infinite error in an unforeseen case, and we don't want to cause performance issues.

However, #14615 encourages us to consider just the case of 1-level deep nested paths. If we cut off at 1 level, we don't need to worry about infinite recursion and risk of performance degradation is much lower.

This PR adds a reasonably simple WithLevel1NestedPaths helper which, given a type, also adds all the 1-level deep nested paths to the result type. That makes it easy to get the correct result type for distinct() with 1-level deep nested paths. This PR intentionally avoids arrays.

Examples

@hasezoey hasezoey added the typescript Types or Types-test related issue / Pull Request label Jun 2, 2024
Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vkarpov15 vkarpov15 added this to the 8.5 milestone Jun 4, 2024
@vkarpov15 vkarpov15 changed the base branch from master to 8.5 June 4, 2024 22:32
@vkarpov15
Copy link
Collaborator Author

Merging this into 8.5 branch in the interest of caution

@vkarpov15 vkarpov15 merged commit 6183560 into 8.5 Jun 4, 2024
5 checks passed
@hasezoey hasezoey deleted the vkarpov15/gh-14615 branch June 5, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type is incorrect for Query.distinct when field is a nested field
2 participants