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

distinct return ts type is unclear #11306

Closed
avichai-sealights opened this issue Feb 1, 2022 · 4 comments
Closed

distinct return ts type is unclear #11306

avichai-sealights opened this issue Feb 1, 2022 · 4 comments
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@avichai-sealights
Copy link

avichai-sealights commented Feb 1, 2022

Do you want to request a feature or report a bug?
Bug(?)
What is the current behavior?
distinct returns an array of Query doc type

const schema = new Schema<IModelEntry>({
    field: String
})
const model: Model<IModelEntry> = this.connection.model<IModelEntry>("collection", schema);
const response = await model.distinct('field', {});

According to the docs: distinct returns the distinct values of the given field that match filter. So if field is a string, a string[] should return.
However response is of type IModelEntry[]

What is the expected behavior?
distinct returns array of string
What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
16.11, 5.13.14, 5.0.4

@IslandRhythms IslandRhythms added the typescript Types or Types-test related issue / Pull Request label Feb 3, 2022
@IslandRhythms
Copy link
Collaborator

returns as type any[]

import * as mongoose from 'mongoose';

interface IModelEntry {
    field: string
}
const schema = new mongoose.Schema<IModelEntry>({
    field: String
})
const model: mongoose.Model<IModelEntry> = mongoose.model<IModelEntry>("collection", schema);

async function test() {
    const response = await model.distinct('field', {});
}

test();

@vkarpov15 vkarpov15 added this to the 6.2.4 milestone Feb 6, 2022
@vkarpov15
Copy link
Collaborator

Confirmed that the return type of await model.distinct('field', {}); is any[]. Which is not incorrect - technically distinct() can return an array of mixed types for field because there's nothing stopping you from inserting a document with field = null or field = true from the mongo shell.

We'll add the ability to do await model.distinct<string>('field') to make distinct() resolve to string[] for v6.2.4 to work around this.

@kaulsh
Copy link
Contributor

kaulsh commented Aug 30, 2023

@vkarpov15 I was thinking the correct type of distinct() should be something like this:

distinct<DocKey extends string, Result = DocKey extends keyof TRawDocType ? TRawDocType[DocKey] : unknown>(
        field: DocKey,
        filter?: FilterQuery<TRawDocType>
    ): QueryWithHelpers<
      Array<Result>,
      THydratedDocumentType,
      TQueryHelpers,
      TRawDocType,
      'distinct'
    >;

This should behave more like mongodb does, i.e., if you give something that isn't a key on the document, you still get an empty array or something unknown[]. With my suggested type, if you do give the correct key from the schema, you'll get a typesafe result.

If you're okay with this type, I'd be happy to raise a PR with tests.

@vkarpov15
Copy link
Collaborator

@kaulshashank looks good as long as tests pass. Feel free to put in a PR, we'll review it. My only potential concern is that this change will be backwards breaking; but if it is, we can always merge into the 8.0 branch.

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

No branches or pull requests

4 participants