TS 5.4.2 breaks `ConditionalKeys` and `SetParameterType` about type-fest

[trevorade]

Hey @voxpelli, we were just updating our code repository in my company to 5.4.2 and this test started breaking so I figured I would send an early signal. I'm not really looking for contribution opportunities at the moment.

Here's the ConditionalKeys test rewritten in TS Playground: link (change the version to "5.3.3" to see the old behavior)

[shicks]

How come the canary is not picking up on this? https://github.com/sindresorhus/type-fest/actions/workflows/ts-canary.yml

We don't use tsd in our internal fork, but have (programmatically) rewritten the tests to use a simpler typechecking assertion library. My guess about why your CI workflow didn't pick up the breakage is that it was using tsd for type checking, which has its own version of typescript built into it - so it's still effectively testing against the old version. (FWIW, this is the main reason we don't use tsd - we want a single typescript import to control all type checking everywhere).

[sindresorhus]

// @ifiokjr @Emiyaaaaa

[voxpelli]

How come the canary is not picking up on this? https://github.com/sindresorhus/type-fest/actions/workflows/ts-canary.yml

[voxpelli]

@trevorade Extending the tests to highlight this failure would be a good first step I think 👍

This is the second breakage in the 5.4-line, after #784

[trevorade]

For ConditionalKeys, looks like the fix is to just add -? and drop the NonNullable:

export type ConditionalKeys<Base, Condition> = {
  // Map through all the keys of the given base type.
  [Key in keyof Base]-?:
  // Pick only keys with types extending the given `Condition` type.
  Base[Key] extends Condition
  // Retain this key since the condition passes.
    ? Key
  // Discard this key since the condition fails.
    : never;

  // Convert the produced object into a union type of the keys which passed the conditional test.
}[keyof Base];

Fixes the tests on our end anyways.

[trevorade]

Here's some debugging info for SerParameterType. Specifically, the issue appears to be with MergeObjectToTuple:

TS Playground link (change the version to 5.3.3 to see the difference)

This one may indicate a TypeScript bug

[trevorade]

Simpler repro of the underlying issue.

TS Playground link (change the version to 5.3.3 to see the difference)

I'll see about logging an issue.

[Emiyaaaaa]

Hi @trevorade

I did some test, If we focus only on the results of type A, it looks like 5.4.2 is correct?

But whatever this is a breaking change..

type Test<Tuple extends unknown[]> = {
	[K in keyof Tuple]: K
};

type A = Test<[a: number, b: string, c: Object, ...arguments: boolean[]]>
// v5.3.3 type A = [a: "0", b: "1", c: "2", ...arguments: "3"[]]
// v5.4.2 type A = [a: "0", b: "1", c: "2", ...arguments: number[]]

Playground Link

[trevorade]

Looks like SetParameterType is failing for rest arguments due to this TSC PR: microsoft/TypeScript#57031

I haven't looked closer to see what the correct rewrite would be.

[sindresorhus]

SetParameterType should be fixed in https://github.com/sindresorhus/type-fest/releases/tag/v4.13.0

[trevorade]

Thanks. I'll verify the fix on our end later today

[trevorade]

Confirmed on our end that SetParameterType is fixed. Thx! [See update below]

ConditionalKeys should be easy to fix:
#831 (comment)

[trevorade]

Apologies, SetParameterType is not quite fixed.

For this case in tsc 5.4.2:

// Test another define way
declare const test3: SetParameterType<typeof fn, [a: string, b: boolean]>;
expectType<(a: string, b: boolean, c: Object, ...args: boolean[]) => null>(test3);
test3('1', true, {}, true);

I'm seeing the type of test3 resolve to:

type test3Type = (_a: string, _b: boolean, _c: Object, ..._arguments: (string | boolean)[]) => null

You can ignore the difference in parameter names but the last rest parameter is resolving to (string | boolean)[] rather than the expected boolean[].

Here's a reproduction of this error in TS Playground.

[Emiyaaaaa]

Sorry, I made a mistake that ignore the number key when input is an array. I'm fixing it right now.

It seems that the improvement of the testing system is more urgent.

[sindresorhus]

It seems that the improvement of the testing system is more urgent.

#837