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] Adjusments to ApplyBasicQueryCasting (ff2bf3e8b3abe0e09a523465e5db8743083702b9) break FilterQueries for generic types #11964

Closed
2 tasks done
ghost91- opened this issue Jun 21, 2022 · 1 comment
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@ghost91-
Copy link
Contributor

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

6.4.0

Node.js version

16.x

MongoDB server version

5.x

Description

The changes from ff2bf3e break FilterQuerys for generic types that have some know properties. With this change, it's not possible anymore to assign values for the known properties.

It seems like the issue is the T[0] & in

  export type ApplyBasicQueryCasting<T, defaultT = T | T[] | {}> = T extends any[] ? T[0] & defaultT: defaultT;

What exactly is the purpose of having T[0] & defaultT? For arrays, that would mean the property needs to be of the element type, and the array type or an array of the array type or any non nullable type, at the same time. That doesn't make a lot of sense to me.

Also, I doubt {} is the correct type here. It means "any non nullable value", not "empty object". In particular, you can do stuff like const foo: {} = true. null and undefined are the only values that can not be assigned to it. I doubt that's what's meant here.

From what I can see, the second generic parameter (defaultT) isn't ever set explicitly, so what is the purpose of it?

And lastly, that commit also introduces some tests, but it is in no way clear how the tests are related to the changes. And it seems like they don't cover all parts of the changes. So I think the tests should be expanded a bit here.

Steps to Reproduce

import { ApplyBasicQueryCasting, FilterQuery, QuerySelector } from 'mongoose';

type Condition<T> = ApplyBasicQueryCasting<T> | QuerySelector<ApplyBasicQueryCasting<T>>; // redefined here because it's not exported by mongoose

type WithId<T extends object> = T & { id: string };

class Repository<T extends object> {
  /* ... */

  find(id: string) {
    const idCondition: Condition<WithId<T>>['id'] = id; // error :(
    const filter: FilterQuery<WithId<T>> = { id }; // error :(

    /* ... */
  }
}

Expected Behavior

Using FilterQuery with generic types with know properties works as it has previously.

@IslandRhythms IslandRhythms added the typescript Types or Types-test related issue / Pull Request label Jun 21, 2022
@vkarpov15 vkarpov15 added this to the 6.4.1 milestone Jun 21, 2022
@vkarpov15
Copy link
Collaborator

Thanks for the detailed issue report, you're right that the new implementation of ApplyBasicQueryCasting in v6.4.0 is incorrect. We'll have a fix in v6.4.1.

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

3 participants