types: allow passing in projections without as const
#15564
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses TypeScript typing issues with MongoDB projections in Mongoose by loosening projection type validation. The change allows users to pass projection objects without requiring the as const assertion, which was causing problems for some use cases while maintaining type safety where possible.
- Removes strict validation that previously required
as constfor projection objects - Updates test cases to reflect the more permissive projection typing
- Maintains basic projection validation while allowing more flexible usage patterns
Comments suppressed due to low confidence (3)
test/types/queries.test.ts:158
- This test now allows invalid projection values (3 is not a valid MongoDB projection value). Consider adding a comment explaining why this is now permitted or verify this aligns with the intended behavior change.
Test.find({}, { name: 3 });
test/types/queries.test.ts:159
- This test now allows mixing inclusion and exclusion projection operators, which is invalid in MongoDB. This significant behavior change should be documented to clarify whether this is intentional relaxation of validation.
Test.find({}, { name: true, age: false, endDate: true, tags: 1 });
test/types/queries.test.ts:180
- The comment states 'does not accept any' but the test expects an error for a string value 'taco', while line 158 allows the number 3. This inconsistency in what projection values are accepted should be clarified.
expectError(Test.find({}, { 'docs.id': 'taco' })); // Dot notation should be allowed and does not accept any
There was a problem hiding this comment.
Code looks good to me.
It would be really great if typescript could automatically narrow to as const by default instead of defaulting to wide types. (I think this has issue microsoft/TypeScript#51930)
As long as that is not the case, we either need to enforce as const for users, do something like Narrow in ts-toolbelt (reddit post) or well, revert to less-specific types like in this PR.
(Note that the ts-toolbelt type i just came across and have not used, but looks like it might fit this case somewhat)
|
Thanks for finding ts-toolbelt. Unfortunately it doesn't look like findOne<ResultDoc = THydratedDocumentType>(
filter?: RootFilterQuery<TRawDocType>,
projection?: Function.Narrow<ProjectionType<TRawDocType>> | null,
options?: QueryOptions<TRawDocType> & mongodb.Abortable | null
): QueryWithHelpers<ResultDoc | null, ResultDoc, TQueryHelpers, TRawDocType, 'findOne', TInstanceMethods & TVirtuals>; |
Fix #15557
Re: #13840
Re: #15327
Summary
#13840 made it so that Mongoose requires
as constfor projections defined usingconst projection = { /* fields here */ }. That can cause some problems like in #15557. I'm not 100% convinced we want to make the change in this PR, I'd like to hear @hasezoey and @AbdelrahmanHafez 's thoughtsExamples