feat: TS types for updates with readonly array values#829
feat: TS types for updates with readonly array values#829duncanbeevers wants to merge 2 commits intoplexinc:mainfrom
Conversation
| }, | ||
| }, | ||
| ]; | ||
| ] as const; |
There was a problem hiding this comment.
Can you also still use the bulkWrite() with an inline argument which is not marked as const? Do we have such a test? Let's add one, if not.
There was a problem hiding this comment.
I added a test that restores the original operations declaration, as well as one that tests an inline operations definition (and preserved the as const test, added here)
src/mongodbTypes.ts
Outdated
| $inc?: OnlyFieldsOfType<DeepReadonly<TSchema>, NumericType | undefined>; | ||
| $min?: PaprMatchKeysAndValues<DeepReadonly<TSchema>>; | ||
| $max?: PaprMatchKeysAndValues<DeepReadonly<TSchema>>; | ||
| $mul?: OnlyFieldsOfType<DeepReadonly<TSchema>, NumericType | undefined>; | ||
| $rename?: Record<string, string>; | ||
| $set?: PaprMatchKeysAndValues<TSchema>; | ||
| $setOnInsert?: PaprMatchKeysAndValues<TSchema>; | ||
| $unset?: OnlyFieldsOfType<TSchema, any, '' | 1 | true>; | ||
| $addToSet?: SetFields<TSchema>; | ||
| $pop?: OnlyFieldsOfType<TSchema, readonly any[], -1 | 1>; | ||
| $pull?: PullOperator<TSchema>; | ||
| $push?: PushOperator<TSchema>; | ||
| $pullAll?: PullAllOperator<TSchema>; | ||
| $set?: PaprMatchKeysAndValues<DeepReadonly<TSchema>>; | ||
| $setOnInsert?: PaprMatchKeysAndValues<DeepReadonly<TSchema>>; | ||
| $unset?: OnlyFieldsOfType<DeepReadonly<TSchema>, any, '' | 1 | true>; | ||
| $addToSet?: SetFields<DeepReadonly<TSchema>>; | ||
| $pop?: OnlyFieldsOfType<DeepReadonly<TSchema>, readonly any[], -1 | 1>; | ||
| $pull?: PullOperator<DeepReadonly<TSchema>>; | ||
| $push?: PushOperator<DeepReadonly<TSchema>>; | ||
| $pullAll?: PullAllOperator<DeepReadonly<TSchema>>; | ||
| $bit?: OnlyFieldsOfType< | ||
| TSchema, | ||
| DeepReadonly<TSchema>, |
There was a problem hiding this comment.
All this wrapping seems not DRY at all. Can we optimize this code somehow?
There was a problem hiding this comment.
I didn't see a simple way to do so, though I feel the duplication is pretty well concentrated, living only here where the major tributaries of the update methods feed into the PaprUpdateFilter defintion, and nowhere else.
* Break many expectations out to individual tests
duncanbeevers
force-pushed
the
fix/set-readonly-array
branch
from
3bb1d86 to
fbcb04e
Compare
|
@avaly diff --git a/@types/mongodb.d.ts b/@types/mongodb.d.ts
index c9004704d4..2ce4be1716 100644
--- a/@types/mongodb.d.ts
+++ b/@types/mongodb.d.ts
@@ -36,6 +36,6 @@ declare global {
// This type is extracted from `papr.PaprMatchKeysAndValues`, because that type is too complex for our usage here,
// and it would introduce more than 3 minutes on the `tsc` check times.
export type MongoAttributes<TSchema> = {
- [Property in mongodb.Join<papr.NestedPaths<TSchema, []>, '.'>]?: papr.PropertyType<TSchema, Property>;
+ [Property in mongodb.Join<papr.NestedPaths<TSchema, []>, '.'>]?: Mutable<papr.PropertyType<TSchema, Property>>;
};
} |
avaly
left a comment
avaly
left a comment
There was a problem hiding this comment.
If this is breaking the public interface of the PropertyType, then this commit needs to be marked as having breaking changes.
Please add BREAKING CHANGE: ...details in the commit message.
|
@avaly I've tested it in our internal repos and it is compiling without errors. Should we adapt other methods like insert to support readonly array too? |
|
Thanks for testing @CarlosLozanoHealthCaters! Yeah, we plan to extend support to all methods that accept user input. |
| */ | ||
| export type PaprArrayNestedProperties<TSchema> = { | ||
| [Property in `${KeysOfAType<PaprAllProperties<TSchema>, Record<string, any>[]>}.$${ | ||
| [Property in `${KeysOfAType<PaprAllProperties<TSchema>, readonly Readonly<Record<string, any>>[]>}.$${ |
There was a problem hiding this comment.
Why are there two readonlys here?
There was a problem hiding this comment.
I had the same question but convinced myself it was correct -- I think Readonly makes the record readonly, and then the readonly makes the entire array readonly?
5 participants
🐞 Permit
readonlyarraysRelated issue: #828
Test case demonstrating TS type error.