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: allow arbitrary keys in query filters again #14874

Merged
merged 1 commit into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 1 addition & 27 deletions test/types/models.test.ts
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

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

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
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