Consistently add undefined/missing to optional tuple element types #50831
Conversation
typescript-bot added Author: Team For Uncommitted Bug 
| @typescript-bot test this |
| Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 68f51da. You can monitor the build here. |
| Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 68f51da. You can monitor the build here. Update: The results are in! |
| Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at 68f51da. You can monitor the build here. Update: The results are in! |
| Heya @ahejlsberg, I've started to run the extended test suite on this PR at 68f51da. You can monitor the build here. |
| Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at 68f51da. You can monitor the build here. Update: The results are in! |
typescript-bot added For Milestone Bug 
| @ahejlsberg Here are the results of running the user test suite comparing Everything looks good! |
| @ahejlsberg Here they are:Comparison Report - main..50831
System
Hosts
Scenarios
Developer Information: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Heya @ahejlsberg, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
| @ahejlsberg Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details |
| Debug.assert(strictNullChecks); | ||
| return type.flags & TypeFlags.Undefined ? type : getUnionType([type, isProperty ? missingType : undefinedType]); | ||
| const missingOrUndefined = isProperty ? missingType : undefinedType; | ||
| return type.flags & TypeFlags.Undefined || type.flags & TypeFlags.Union && (type as UnionType).types[0] === missingOrUndefined ? type : getUnionType([type, missingOrUndefined]); |
There was a problem hiding this comment.
The types[0] check always seems extremely fragile, but I suppose it's convenient.
There was a problem hiding this comment.
Yeah, but we make that assumption in several other places, so seems fine to do it here as well.
| @typescript-bot pack this |
| Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 68f51da. You can monitor the build here. |
| Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
| Only one change in the test suites: reduxjs/reselect apparently depends on the ability for mapped types to strip |
| Clearly this is more correct, and we have no examples of breakages in our test suite, across DefinitelyTyped, internal user test, external user test, or the other top 100; but I can't really track what reselect is doing and where inference is falling back to @markerikson @lukeapage, if reselect can be modified to accommodate the existing and new behavior, then that would be ideal. Otherwise, if you can provide a minimal repro, we'd welcome a follow-up issue. |
| Uh. To be honest I really don't have bandwidth to make more changes to the Reselect types any time soon. What we've got there now works, and it took me weeks of effort to get it working correctly because of how complicated the mapped function param type extraction is: https://github.com/reduxjs/reselect/blob/v4.1.5/src/types.ts#L100-L148 I'm going to be pretty unhappy if this gets broken randomly :( |
| If I understood what |
| Okay, lemme see if I can give a really fast example. Reselect accepts multiple "input selectors", and an "output selector": const selectItemsByCategory = createSelector( (state: RootState) => state.items, (state: RootState, category: string) => category, (items, category) => { return items.filter(item => item.category === category); } )At runtime, when you call const extractedValues = inputSelectors.map(input => input(...args)); return outputSelector(...extractedValues);The goal here is:
To do that,
As part of that, passing two input selectors with mismatched arguments should be invalid and result in const selectItemsByCategory = createSelector( (state: RootState) => state.items, (state: RootState, category: string) => category, // ERROR: index 1 here is a number, but `category` says it should be a string! (state: RootState, value: number) => value, (items, category, value) => { return items.filter(item => item.category === category); } ) |
| I think the best step for us to try to bring this change in for the 4.9 beta - that way people can at least try it out easily. If there is no way to resolve the current issues with reselect, we can discuss further options. |
| @markerikson Thanks for the explanation. Below is a simplified version of type UnknownFunction = (...args: any[]) => any; type LongestTuple<T extends readonly unknown[][]> = T extends [infer U extends unknown[]] ? U : T extends [infer U, ...infer R extends unknown[][]] ? MostProperties<U, LongestTuple<R>> : never; type MostProperties<T, U> = keyof U extends keyof T ? T : U; type ElementAt<T extends unknown[], N extends keyof any> = N extends keyof T ? T[N] : unknown; type ElementsAt<T extends readonly unknown[][], N extends keyof any> = { [K in keyof T]: ElementAt<T[K], N> } type Intersect<T extends readonly unknown[]> = T extends [] ? unknown : T extends [infer H, ...infer T] ? H & Intersect<T> : T[number]; type MergeTuples<T extends readonly unknown[][], L extends unknown[] = LongestTuple<T>> = { [K in keyof L]: Intersect<ElementsAt<T, K>> } type ExtractParameters<T extends readonly UnknownFunction[]> = { [K in keyof T]: Parameters<T[K]> } export type MergeParameters<T extends readonly UnknownFunction[]> = "0" extends keyof T ? MergeTuples<ExtractParameters<T>> : Parameters<T[number]>; type Test1 = MergeParameters<[(state: unknown, a: string) => void, (state: unknown) => void, (state: unknown, a: "a" | "b", b?: number) => void]>; type Test2 = MergeParameters<((state: unknown, a: string) => void)[]>; |
| @ahejlsberg Oh wow, thank you! I'll have to take a look and give that a shot. This might require some more |
| Following up on this: @ahejlsberg , yes, that implementation does appear to work right - thank you! (it's impressive how much simpler that implementation of The bad news is that now I have to figure out how to ship this in Reselect and still support users on TS 4.6 and earlier, and that is a separate pain point that I just discussed over in #50196 (comment) . (There's a hacky workaround that involves build-time shenanigans that technically works, but is not a satisfying solution.) |
| I ran into this problem while working on a utility function for combining trait-like functions and implemented a similar type before someone mentioned this one existed. This is just the simplified form for a binary intersection, but could easily be composed together to form a "merge" generic like @ahejlsberg's. There are so many edge cases when it comes to this but it seems to handle most well, e.g. a trailing rest parameter or merging optional and required parameters in the same position. Given how tricky this is to implement at a type-level, would be awesome if TS could compute this directly when intersecting tuples or provide some way to access it. There'd be some nuance since the rules would work differently depending on whether the tuples represented parameters or not, but as is it seems extremely hard (maybe impossible?) to get everything right using types alone, especially when you consider stuff like type intersectParameters< l extends readonly unknown[], r extends readonly unknown[], result extends readonly unknown[] = [] > = l extends readonly [] ? [...result, ...r] : r extends readonly [] ? [...result, ...l] : [number, number] extends [l["length"], r["length"]] ? [...result, ...(l[number] & r[number])[]] : [l, r] extends [ readonly [unknown?, ...infer lTail], readonly [unknown?, ...infer rTail] ] ? intersectParameters<lTail, rTail, [...result, l[0] & r[0]]> : l extends readonly [infer lHead, ...infer lTail] ? intersectParameters<lTail, r, [...result, lHead & r[number]]> : r extends readonly [infer rHead, ...infer rTail] ? intersectParameters<l, rTail, [...result, l[number] & rHead]> : result |
| Welp, I found a few more edge cases, so I made an updated version that's more precise in dealing with optionals, e.g.:
I added some comments this time because things got a little out of hand trying to cover all the edge cases 😆 That said, the underlying logic is quite simple, so it should still be relatively performant: export type intersectParameters< l extends readonly unknown[], r extends readonly unknown[], prefix extends readonly unknown[] = [] > = [parseNextParam<l>, parseNextParam<r>] extends [ infer lState extends ParameterParseResult, infer rState extends ParameterParseResult ] ? false extends lState["done"] | rState["done"] ? // while either array still has fixed elements remaining, continue intersecting heads intersectParameters< lState["tail"], rState["tail"], [ ...prefix, // the intersection is optional only if both elements are optional ...(lState["optional"] | rState["optional"] extends true ? [andPreserveUnknown<lState["head"], rState["head"]>?] : [andPreserveUnknown<lState["head"], rState["head"]>]) ] > : // once both arrays have reached their fixed end or a variadic element, return the final result [ ...prefix, ...(lState["tail"] extends readonly [] ? rState["tail"] extends readonly [] ? [] : // if done and non-empty, we've reached a variadic element // (or it's just a normal array, since number[] === [...number[]]) rState["tail"] : rState["tail"] extends readonly [] ? lState["tail"] : // if we've reached a variadic element in both arrays, intersect them andPreserveUnknown<lState["head"], rState["head"]>[]) ] : never type ParameterParseResult = { head: unknown optional: boolean tail: readonly unknown[] done: boolean } type parseNextParam<params extends readonly unknown[]> = params extends readonly [] ? { // Since a longer args array is assignable to a shorter one, // elements after the last are typed as unknown. For a normal // tuple (not parameters), this would be never. head: unknown optional: true tail: [] done: true } : params extends readonly [(infer head)?, ...infer tail] ? [tail, params] extends [params, tail] ? { head: head optional: true tail: tail done: true } : { head: head // the parameter is required iff its type is the same as the // one we inferred from within (infer head)?, otherwise optional optional: params[0] extends head ? false : true tail: tail done: false } : never type evaluate<t> = { [k in keyof t]: t[k] } & unknown type andPreserveUnknown<l, r> = unknown extends l & r ? unknown : evaluate<l & r> |
5 participants
Fixes #50753.