Consistently add undefined/missing to optional tuple element types

[ahejlsberg]

Fixes #50753.

[ahejlsberg]

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster
@typescript-bot test top100

[typescript-bot]

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 68f51da. You can monitor the build here.

[typescript-bot]

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!

[typescript-bot]

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!

[typescript-bot]

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 68f51da. You can monitor the build here.

[typescript-bot]

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]

@ahejlsberg Here are the results of running the user test suite comparing main and refs/pull/50831/merge:

Everything looks good!

[typescript-bot]

@ahejlsberg
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..50831

Metric	main	50831	Delta	Best	Worst
Angular - node (v14.15.1, x64)
Memory used	338,931k (± 0.01%)	338,926k (± 0.01%)	-5k (- 0.00%)	338,879k	338,975k
Parse Time	2.07s (± 1.18%)	2.06s (± 0.75%)	-0.01s (- 0.68%)	2.04s	2.10s
Bind Time	0.79s (± 0.60%)	0.79s (± 0.63%)	+0.00s (+ 0.25%)	0.79s	0.81s
Check Time	5.83s (± 0.28%)	5.85s (± 0.53%)	+0.02s (+ 0.34%)	5.80s	5.92s
Emit Time	6.18s (± 0.66%)	6.17s (± 0.68%)	-0.01s (- 0.18%)	6.06s	6.29s
Total Time	14.88s (± 0.41%)	14.87s (± 0.49%)	-0.00s (- 0.01%)	14.70s	15.08s
Compiler-Unions - node (v14.15.1, x64)
Memory used	190,154k (± 0.01%)	190,147k (± 0.02%)	-7k (- 0.00%)	190,066k	190,214k
Parse Time	0.85s (± 0.55%)	0.86s (± 0.69%)	+0.01s (+ 0.71%)	0.84s	0.87s
Bind Time	0.48s (± 0.98%)	0.48s (± 0.70%)	0.00s ( 0.00%)	0.48s	0.49s
Check Time	6.69s (± 0.36%)	6.70s (± 0.32%)	+0.01s (+ 0.10%)	6.65s	6.73s
Emit Time	2.37s (± 0.98%)	2.37s (± 1.05%)	+0.00s (+ 0.04%)	2.33s	2.43s
Total Time	10.39s (± 0.21%)	10.41s (± 0.27%)	+0.02s (+ 0.14%)	10.35s	10.49s
Monaco - node (v14.15.1, x64)
Memory used	326,575k (± 0.01%)	326,572k (± 0.01%)	-2k (- 0.00%)	326,522k	326,616k
Parse Time	1.58s (± 0.71%)	1.58s (± 0.73%)	+0.00s (+ 0.13%)	1.56s	1.62s
Bind Time	0.72s (± 0.77%)	0.73s (± 1.20%)	+0.01s (+ 1.11%)	0.72s	0.76s
Check Time	5.70s (± 0.26%)	5.70s (± 0.44%)	+0.00s (+ 0.00%)	5.67s	5.76s
Emit Time	3.30s (± 0.37%)	3.31s (± 0.39%)	+0.01s (+ 0.30%)	3.27s	3.33s
Total Time	11.31s (± 0.21%)	11.33s (± 0.26%)	+0.02s (+ 0.19%)	11.28s	11.40s
TFS - node (v14.15.1, x64)
Memory used	289,701k (± 0.01%)	289,691k (± 0.01%)	-11k (- 0.00%)	289,652k	289,777k
Parse Time	1.30s (± 0.83%)	1.30s (± 0.97%)	-0.00s (- 0.00%)	1.28s	1.34s
Bind Time	0.79s (± 0.51%)	0.79s (± 0.86%)	+0.01s (+ 0.63%)	0.78s	0.81s
Check Time	5.33s (± 0.35%)	5.34s (± 0.43%)	+0.01s (+ 0.11%)	5.30s	5.39s
Emit Time	3.53s (± 0.72%)	3.53s (± 0.69%)	-0.00s (- 0.06%)	3.48s	3.58s
Total Time	10.95s (± 0.35%)	10.96s (± 0.27%)	+0.01s (+ 0.05%)	10.87s	11.01s
material-ui - node (v14.15.1, x64)
Memory used	436,692k (± 0.00%)	436,667k (± 0.00%)	-25k (- 0.01%)	436,641k	436,706k
Parse Time	1.87s (± 0.50%)	1.87s (± 0.45%)	+0.00s (+ 0.05%)	1.86s	1.90s
Bind Time	0.58s (± 0.81%)	0.58s (± 0.63%)	+0.00s (+ 0.86%)	0.58s	0.59s
Check Time	12.81s (± 0.32%)	12.85s (± 0.35%)	+0.04s (+ 0.34%)	12.75s	12.97s
Emit Time	0.00s (± 0.00%)	0.00s (± 0.00%)	0.00s ( NaN%)	0.00s	0.00s
Total Time	15.26s (± 0.28%)	15.31s (± 0.32%)	+0.05s (+ 0.31%)	15.21s	15.42s
xstate - node (v14.15.1, x64)
Memory used	546,998k (± 0.00%)	546,953k (± 0.00%)	-45k (- 0.01%)	546,920k	546,982k
Parse Time	2.60s (± 0.42%)	2.61s (± 0.44%)	+0.01s (+ 0.42%)	2.59s	2.64s
Bind Time	0.98s (± 1.10%)	0.97s (± 0.78%)	-0.00s (- 0.41%)	0.96s	0.99s
Check Time	1.52s (± 0.34%)	1.51s (± 0.60%)	-0.01s (- 0.66%)	1.50s	1.54s
Emit Time	0.07s (± 0.00%)	0.07s (± 0.00%)	0.00s ( 0.00%)	0.07s	0.07s
Total Time	5.17s (± 0.26%)	5.18s (± 0.36%)	+0.01s (+ 0.17%)	5.14s	5.23s

System

Machine Name	ts-ci-ubuntu
Platform	linux 4.4.0-210-generic
Architecture	x64
Available Memory	16 GB
Available Memory	15 GB
CPUs	4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz

Hosts

• node (v14.15.1, x64)

Scenarios

• Angular - node (v14.15.1, x64)
• Compiler-Unions - node (v14.15.1, x64)
• Monaco - node (v14.15.1, x64)
• TFS - node (v14.15.1, x64)
• material-ui - node (v14.15.1, x64)
• xstate - node (v14.15.1, x64)

Benchmark	Name	Iterations
Current	50831	10
Baseline	main	10

Developer Information:

Download Benchmark

[typescript-bot]

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.

[typescript-bot]

@ahejlsberg Here are the results of running the top-repos suite comparing main and refs/pull/50831/merge:

Something interesting changed - please have a look.

Details

reduxjs/reselect

1 of 3 projects failed to build with the old tsc and were ignored

typescript_test/tsconfig.json

• error TS2578: Unused '@ts-expect-error' directive.
  • typescript_test/test.ts#L677
  • typescript_test/test.ts#L1621
• error TS2345: Argument of type '[{ counter1: number; counter2: number; }, ({} | undefined)?]' is not assignable to parameter of type '[State, (number | undefined)?]'.
  • typescript_test/test.ts#L1604

[DanielRosenwasser]

The types[0] check always seems extremely fragile, but I suppose it's convenient.

[ahejlsberg]

Yeah, but we make that assumption in several other places, so seems fine to do it here as well.

[DanielRosenwasser]

@typescript-bot pack this

[typescript-bot]

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 68f51da. You can monitor the build here.

[typescript-bot]

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/134597/artifacts?artifactName=tgz&fileId=41FC835DD88D5128FF55423635F1054535236518B88F79F131084E4C4E03AAAC02&fileName=/typescript-4.9.0-insiders.20220919.tgz"
    }
}

and then running npm install.

---

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.9.0-pr-50831-12".;

[ahejlsberg]

Only one change in the test suites: reduxjs/reselect apparently depends on the ability for mapped types to strip undefined from the types of optional properties without stripping optionality at the same time. Since such types are demonstrably incorrect I think this break is acceptable.

[DanielRosenwasser]

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 {} in those tests.

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

[markerikson]

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

[ahejlsberg]

If I understood what MergeParameters is doing, I'd be happy to think about an implementation that doesn't break.

[markerikson]

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 items = selectItemsByCategory(state, "food"), it maps over each input selector and calls it, like:

const extractedValues = inputSelectors.map(input => input(...args));
return outputSelector(...extractedValues);

The goal here is:

• Determine the type of the parameters for each input selector
• Group them by index ("all params at index 0", "all params at index 1")
• For each group, intersect them together so that we have the tightest possible type that is valid at that param index
• Generate a new selector that uses those types as its arguments

To do that, MergeParamaters:

• Gets "the type of each input selector"
• Gets "the type of each input selector's params"
• Shuffles them to group by param index
• For each index, runs a recursive intersection across the params at that index
• Turns all that back into [typeForIndex0, typeForIndex1, typeForIndex2], etc

As part of that, passing two input selectors with mismatched arguments should be invalid and result in unknown, like:

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);
  }
)

[DanielRosenwasser]

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.

[ahejlsberg]

@markerikson Thanks for the explanation. Below is a simplified version of MergeParameters that works with TS version 4.7 and later. Hope this helps.

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)[]>;

[markerikson]

@ahejlsberg Oh wow, thank you! I'll have to take a look and give that a shot. This might require some more typesVersions fiddling, but really appreciate the help!

[markerikson]

Following up on this:

@ahejlsberg , yes, that implementation does appear to work right - thank you!

(it's impressive how much simpler that implementation of MergeParameters is than the one we had. Took me weeks to get that one working, and that was with tons of assistance from other TS wizards much smarter than me :) )

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

[ssalbdivad]

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 (state: unknown, ...rest: [...string[], number]) => void.

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

[ssalbdivad]

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>