Skip to content

Commit bbb1a6b

Browse files
committed
Improve type safety for recursive objects
With the removal of the `{ [key: string]: any }` extension on the `Filter<TSchema>` type, we find some serious new shortcomings on the handling of Recursive types: they break type safety for some non-recursive types! Failing Example =============== Consider the case in the tests with the following schema: ``` interface TypedDoc { name: string; age: number; listOfNumbers: number[]; tag: { name: string; }; } ``` The way we were detecting recursion was to check if, for a given `Type extends Object` and a `Key` in that object, if `Type extends Type[Key]`. Note that in the `TypedDoc` interface there is no recursive structure, but that `TypedDoc extends TypedDoc['tag']` is true, because the set of keys in `TypedDoc` is a superset of the keys in `TypedDoc['tag']`. This meant that, for this simple schema, the `tag.name` property was considered to be within a recursion, and so was not getting properly type checked in filtering. Solution Explanation ==================== I've added a new generic type helper, `isInUnion<A,B>` that checks if `A` is a union type that includes `B`. We start by using the `Extract` utility to find the set of types in `A` that are assignable to `B`. If the only overlapping entry is `B` itself, then either: - `A` and `B` are the same type, or - `A` is a union type that includes `B`. Of course, testing equality of Types is also not trivial; there is some discussion at microsoft/TypeScript#27024, with a solution by [@mattmccutchen](https://github.com/mattmccutchen). This should be enough to differentiate when we have recursive structures, even when the recursive part is inside a union. Downsides ========= There are several test cases for recursive data structures that become more cumbersome with the improved type checking. Where the earlier type check would allow querying with deeply nested recursive values, the stricter type checks now added would require that the query author annotate the key with a `@ts-expect-error` directive to opt-out of the type checker. This is likely to be somewhat irksome for folks who commonly write these types of queries, but I believe that this is preferable to the loss of type safety that used to exist (as these were implicitly matching an `any` value). Follow Ups ========== I suspect that it is possible to write a much simpler version of the `NestedPaths` helper; for example, [@jcalz](https://github.com/jcalz) offers a much tighter implementation in https://stackoverflow.com/questions/58434389/typescript-deep-keyof-of-a-nested-object/58436959#58436959 that uses a maximum depth to allow traversing recursive data structures up to a given limit. This kind of implementation would improve the type safety of recursive data structures at the cost of limiting the depth of query nodes. I'm not in a position to understand the common usage of the library, so I'm leaving this alone for future contributors to consider.
1 parent e012ae6 commit bbb1a6b

File tree

3 files changed

+31
-8
lines changed

3 files changed

+31
-8
lines changed

src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ export type {
284284
InferIdType,
285285
IntegerType,
286286
IsAny,
287+
IsInUnion,
287288
Join,
288289
KeysOfAType,
289290
KeysOfOtherType,
@@ -306,6 +307,7 @@ export type {
306307
RootFilterOperators,
307308
SchemaMember,
308309
SetFields,
310+
TypeEquals,
309311
UpdateFilter,
310312
WithId,
311313
WithoutId

src/mongo_types.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,24 @@ export type PropertyType<Type, Property extends string> = string extends Propert
481481
: unknown
482482
: unknown;
483483

484+
/**
485+
* Check if two types are exactly equal
486+
*
487+
* From https://github.com/microsoft/TypeScript/issues/27024#issuecomment-421529650,
488+
* credit to https://github.com/mattmccutchen.
489+
* @public
490+
*/
491+
// prettier-ignore
492+
export type TypeEquals<X, Y> =
493+
(<T>() => T extends X ? 1 : 2) extends
494+
(<T>() => T extends Y ? 1 : 2) ? true : false
495+
496+
/**
497+
* Check if A is a union type that includes B
498+
* @public
499+
*/
500+
export type IsInUnion<A, B> = TypeEquals<Extract<A, B>, B>;
501+
484502
/**
485503
* @public
486504
* returns tuple of strings (keys to be joined on '.') that represent every path into a schema
@@ -506,9 +524,7 @@ export type NestedPaths<Type> = Type extends
506524
? {
507525
[Key in Extract<keyof Type, string>]: Type[Key] extends Type // type of value extends the parent
508526
? [Key]
509-
: // for a recursive union type, the child will never extend the parent type.
510-
// but the parent will still extend the child
511-
Type extends Type[Key]
527+
: IsInUnion<Type[Key], Type> extends true
512528
? [Key]
513529
: Type[Key] extends ReadonlyArray<infer ArrayType> // handling recursive types with arrays
514530
? Type extends ArrayType // is the type of the parent the same as the type of the array?

test/types/community/collection/findX-recursive-types.test-d.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ recursiveOptionalCollection.find({
8787
* recursive union types are supported
8888
*/
8989
interface Node {
90+
value?: string;
9091
next: Node | null;
9192
}
9293

@@ -103,10 +104,12 @@ expectError(
103104
);
104105

105106
nodeCollection.find({
106-
'next.next': 'asdf'
107+
'next.value': 'asdf'
107108
});
108109

109-
nodeCollection.find({ 'next.next.next': 'yoohoo' });
110+
// type safety is lost through recursive relations; callers will
111+
// need to annotate queries with `ts-expect-error` comments
112+
expectError(nodeCollection.find({ 'next.next.value': 'yoohoo' }));
110113

111114
/**
112115
* Recursive schemas with arrays are also supported
@@ -149,9 +152,11 @@ expectError(
149152

150153
// type safety breaks after the first
151154
// level of nested types
152-
recursiveSchemaWithArray.findOne({
153-
'branches.0.directories.0.files.0.id': 'hello'
154-
});
155+
expectError(
156+
recursiveSchemaWithArray.findOne({
157+
'branches.0.directories.0.files.0.id': 'hello'
158+
})
159+
);
155160

156161
recursiveSchemaWithArray.findOne({
157162
branches: [

0 commit comments

Comments
 (0)