Skip to content

types: allow arbitrary keys in query filters again #14874

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 10, 2024
Merged

types: allow arbitrary keys in query filters again #14874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
types: allow arbitrary keys in query filters again 
Revert #14764
Fix #14863
Fix #14862
  • Loading branch information
@vkarpov15
vkarpov15 committed Sep 9, 2024
commit c6af5ec7529a330d5bfb317778e22a3f72925d2a
28 changes: 1 addition & 27 deletions test/types/models.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ function find() {
Project.find({ name: 'Hello' });

// just callback; this is no longer supported on .find()
expectError(Project.find((error: CallbackError, result: IProject[]) => console.log(error, result)));
Project.find((error: CallbackError, result: IProject[]) => console.log(error, result));
Copy link
Collaborator

@hasezoey hasezoey Sep 10, 2024

Choose a reason for hiding this comment

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

after the revert, does the "expectError" now fail?

Copy link
Collaborator Author

@vkarpov15 vkarpov15 Sep 10, 2024

Choose a reason for hiding this comment

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

Yes. Functions match "object with any keys" in TypeScript, and telling TypeScript "this parameter can be any non-function object" is tricky, I haven't been able to figure out how to do that.

image

I would love to keep the "this parameter can't be a function" behavior, but not sure how that would work


// filter + projection
Project.find({}, undefined);
Expand Down Expand Up @@ -977,29 +977,3 @@ function testWithLevel1NestedPaths() {
'foo.one': string | null | undefined
}>({} as Test2);
}

function gh14764TestFilterQueryRestrictions() {
const TestModel = model<{ validKey: number }>('Test', new Schema({}));
// A key not in the schema should be invalid
expectError(TestModel.find({ invalidKey: 0 }));
// A key not in the schema should be invalid for simple root operators
expectError(TestModel.find({ $and: [{ invalidKey: 0 }] }));

// Any "nested" keys should be valid
TestModel.find({ 'validKey.subkey': 0 });

// And deeply "nested" keys should be valid
TestModel.find({ 'validKey.deep.nested.key': 0 });
TestModel.find({ validKey: { deep: { nested: { key: 0 } } } });

// Any Query should be accepted as the root argument (due to merge support)
TestModel.find(TestModel.find());
// A Query should not be a valid type for a FilterQuery within an op like $and
expectError(TestModel.find({ $and: [TestModel.find()] }));

const id = new Types.ObjectId();
// Any ObjectId should be accepted as the root argument
TestModel.find(id);
// A ObjectId should not be a valid type for a FilterQuery within an op like $and
expectError(TestModel.find({ $and: [id] }));
}
8 changes: 3 additions & 5 deletions types/query.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ declare module 'mongoose' {
*/
type RootFilterQuery<T> = FilterQuery<T> | Query<any, any> | Types.ObjectId;

type FilterQuery<T> ={
type FilterQuery<T> = {
[P in keyof T]?: Condition<T[P]>;
} & RootQuerySelector<T> & { _id?: Condition<string>; };

Expand Down Expand Up @@ -117,10 +117,8 @@ declare module 'mongoose' {
/** @see https://www.mongodb.com/docs/manual/reference/operator/query/comment/#op._S_comment */
$comment?: string;
$expr?: Record<string, any>;
// we could not find a proper TypeScript generic to support nested queries e.g. 'user.friends.name'
// this will mark all unrecognized properties as any (including nested queries) only if
// they include a "." (to avoid generically allowing any unexpected keys)
[nestedSelector: `${string}.${string}`]: any;
// this will mark all unrecognized properties as any (including nested queries)
[key: string]: any;
};

interface QueryTimestampsConfig {
Expand Down