v4.0.x -> v4.1.1-rc change in overload selection

[tchetwin]

TypeScript Version: 4.1.0-dev.20201029, 4.1.1-rc

Search Terms: overload

Code

import { promisify } from "util";

// These are components of @types/xml2js@0.4.5
export interface convertableToString {
    toString(): string;
}
declare class Parser {
    parseString(str: convertableToString, cb?: Function): void;
}

const xmlParser = new Parser();
const parseString = xmlParser.parseString.bind(xmlParser);
const parseStringP = promisify(parseString); // <--- interested in overload selection from this line

parseStringP("abc");
//           ^--- error surfaces here

Expected behavior:

No error. This overload is selected from promisify:

(alias) promisify<convertableToString, unknown>(fn: (arg1: convertableToString, callback: (err: any, result: unknown) => void) => void): (arg1: convertableToString) => Promise<...> (+13 overloads)

Actual behavior:

Expected 0 arguments, but got 1. (2554) (on the argument to parseStringP). This overload is selected from promisify:

(alias) promisify<unknown>(fn: (callback: (err: any, result: unknown) => void) => void): () => Promise<unknown> (+13 overloads)

Notes:

Overloads from unpkg:

interface CustomPromisifyLegacy<TCustom extends Function> extends Function {
    __promisify__: TCustom;
}
interface CustomPromisifySymbol<TCustom extends Function> extends Function {
    [promisify.custom]: TCustom;
}
type CustomPromisify<TCustom extends Function> = CustomPromisifySymbol<TCustom> | CustomPromisifyLegacy<TCustom>;

function promisify<TCustom extends Function>(fn: CustomPromisify<TCustom>): TCustom;
function promisify<TResult>(fn: (callback: (err: any, result: TResult) => void) => void): () => Promise<TResult>;
function promisify(fn: (callback: (err?: any) => void) => void): () => Promise<void>;
function promisify<T1, TResult>(fn: (arg1: T1, callback: (err: any, result: TResult) => void) => void): (arg1: T1) => Promise<TResult>;
function promisify<T1>(fn: (arg1: T1, callback: (err?: any) => void) => void): (arg1: T1) => Promise<void>;
function promisify<T1, T2, TResult>(fn: (arg1: T1, arg2: T2, callback: (err: any, result: TResult) => void) => void): (arg1: T1, arg2: T2) => Promise<TResult>;
function promisify<T1, T2>(fn: (arg1: T1, arg2: T2, callback: (err?: any) => void) => void): (arg1: T1, arg2: T2) => Promise<void>;
// removed overloads with 3, 4 & 5 template parameters for brevity
function promisify(fn: Function): Function;

Playground Links:

4.1.0-beta ✔️
4.1.0-dev.20201028 ✔️
4.1.0-dev.20201029 ❌
4.1.1-rc ❌
4.2.0-dev.20201112 ❌

Related Issues:

#41099 - Similar looking but the version introducing the regression doesn't align.
#41359 (comment) - Previously reported in this Issue

[robpalme]

@DanielRosenwasser This is the late-breaking 4.1 compatibility break I mentioned earlier. It didn't really hamper the upgrade because it was easy to circumvent.

[DanielRosenwasser]

@RyanCavanaugh let's discuss about this briefly before the release. It sounds like it's not a blocker, but I can't make sense of the change here and it affects the inference of Node.js's promisify.

[DanielRosenwasser]

@ahejlsberg too - unclear if #41099 is related.

[DanielRosenwasser]

@ahejlsberg I believe this regression came from #41308

Here's an easier diff view https://github.com/microsoft/TypeScript/pull/41308/files?w=1

[DanielRosenwasser]

Okay, here is the minimal repro:

declare function promisify<TResult>(fn: (callback: (err: any, result: TResult) => void) => void): () => Promise<TResult>;
declare function promisify<T1, TResult>(fn: (arg1: T1, callback: (err: any, result: TResult) => void) => void): (arg1: T1) => Promise<TResult>;

declare function parseString(str: any, cb?: Function): void;

const parseStringP = promisify(parseString);

let x = parseStringP("abc");

This works in TypeScript 4.0, but not in 4.1 or after that PR. If you swap the overloads, then things work again.

---

Edit: slightly better repro here

declare function parseString(str: any, cb?: (...args: any) => any): void;

const parseStringP = promisify(parseString);

let x = parseStringP("abc");

const arg1UnknownError: (arg: string) => Promise<number> = promisify((arg: string, cb: (err: unknown, result: number) => void): void => { });

declare function promisify<TResult>(fn: (callback: (err: any, result: TResult) => void) => void): () => Promise<TResult>;
declare function promisify<T1, TResult>(fn: (arg1: T1, callback: (err: any, result: TResult) => void) => void): (arg1: T1) => Promise<TResult>;

[DanielRosenwasser]

I tried swapping each of the corresponding void/non-void callback operators, let's see if it can just be a DefinitelyTyped fix: DefinitelyTyped/DefinitelyTyped#49640

[ahejlsberg]

I took a look. Short story, the current behavior is correct and the previous behavior wasn't. But it is incredibly subtle. It comes down to relating the types

• source (str: convertableToString, cb?: Function | undefined) => void and
• target (cb: (err: any, result: unknown) => void) => void

during overload resolution. First subtlety is that convertableToString is effectively equivalent to {} because every object has a toString(): string method. So, the cb and str parameters end up being compatible, and since target only supplies one parameter there is no need to relate the cb? parameter to anything. Overload resolution therefore succeeds and picks an overload that subsequently causes an error.

I didn't succeed before because we'd relate type Function | undefined to any for the second parameter instead of not relating at all. The intent here was to have it always succeed (because anything is assignable to any). However, the first pass of overload resolution uses the subtype relation, and in that relation any is a top type and not assignable to Function | undefined. So, overload resolution would (erroneously) skip the overload.

Like I said, subtle!

The real issue is that the overloads aren't selective enough and/or listed in the wrong order.

[DanielRosenwasser]

I don't think there's an obvious way to reorder promisify in a way that doesn't introduce a new break. Here's a few test cases included:

declare function parseString(str: any, cb?: (...args: any) => any): void;

const parseStringP = promisify(parseString);

let x = parseStringP("abc");

const arg1UnknownError: (arg: string) => Promise<number> = promisify((arg: string, cb: (err: unknown, result: number) => void): void => { });
const arg1AnyError: (arg: string) => Promise<number> = promisify((arg: string, cb: (err: any, result: number) => void): void => { });

const arg0: () => Promise<number> = promisify((cb: (err: Error | null, result: number) => void): void => { });
const arg0NoResult: () => Promise<any> = promisify((cb: (err: Error | null) => void): void => { });
const arg1: (arg: string) => Promise<number> = promisify((arg: string, cb: (err: Error | null, result: number) => void): void => { });
const arg1UnknownError2: (arg: string) => Promise<number> = promisify((arg: string, cb: (err: Error | null, result: number) => void): void => { });
const arg1NoResult: (arg: string) => Promise<any> = promisify((arg: string, cb: (err: Error | null) => void): void => { });
const cbOptionalError: () => Promise<void | {}> = promisify((cb: (err?: Error | null) => void): void => { cb(); }); // tslint:disable-line void-return

declare function promisify<TResult>(fn: (callback: (err: any, result: TResult) => void) => void): () => Promise<TResult>;
declare function promisify(fn: (callback: (err?: any) => void) => void): () => Promise<void>;
declare function promisify<T1, TResult>(fn: (arg1: T1, callback: (err: any, result: TResult) => void) => void): (arg1: T1) => Promise<TResult>;
declare function promisify<T1>(fn: (arg1: T1, callback: (err?: any) => void) => void): (arg1: T1) => Promise<void>;
declare function promisify<T1, T2, TResult>(fn: (arg1: T1, arg2: T2, callback: (err: any, result: TResult) => void) => void): (arg1: T1, arg2: T2) => Promise<TResult>;
declare function promisify<T1, T2>(fn: (arg1: T1, arg2: T2, callback: (err?: any) => void) => void): (arg1: T1, arg2: T2) => Promise<void>;

I could imagine trying a migration to rest parameter types, but I'd rather not do that for this change.

[DanielRosenwasser]

@tchetwin @robpalme you mentioned that this was something you managed to get around - do you remember what you specifically did?

[tchetwin]

@tchetwin @robpalme you mentioned that this was something you managed to get around - do you remember what you specifically did?

By as ... casting the argument to promisify to force selection of the correct overload.

This occurred in precisely one place and it happened to be in the shared build tooling itself rather than the application population.

[AlCalzone]

I could imagine trying a migration to rest parameter types, but I'd rather not do that for this change.

How do you like this?

/**
 * Returns the last item's type in a tuple
 */
export type Last<T extends unknown[]> = T extends []
	? never
	: T extends [...infer _, infer R] ? R
	: T extends [...infer _, (infer R)?] ? (R | undefined)
	: never;

/** Returns the type of the last argument of a function */
export type LastArgument<T extends (...args: any[]) => any> = Last<Parameters<T>>;

/** Returns the "return" type of a callback-style API */
export type CallbackAPIReturnType<
	T extends (...args: any[]) => any,
	TCb extends (...args: any[]) => any = LastArgument<T>,
	TCbArgs = Parameters<Exclude<TCb, undefined>>,
	> =
	TCbArgs extends [(Error | null | undefined)?] ? void :
	TCbArgs extends [Error | null | undefined, infer U] ? U :
	TCbArgs extends any[] ? TCbArgs[1] :
	never;

/**
 * Returns a promisified function signature for the given callback-style function.
 */
export type Promisify<T extends (...args: any[]) => any,
	TReturn = CallbackAPIReturnType<T>,
	TArgs extends any[] = Lead<Parameters<T>>,
	> = (...args: TArgs) => Promise<TReturn>;

/**
 * Returns all but the last item's type in a tuple/array
 */
export type Lead<T extends unknown[]> = T extends []
	? []
	: T extends [...infer L, any?] ? L
	: [];

declare function promisify<T extends (...args: any[]) => any>(fn: T): Promisify<T>;

results for your test cases:

declare function parseString(str: any, cb?: (...args: any[]) => any): void;

const parseStringP = promisify(parseString);
let x = parseStringP("abc");
// has type  (str: any) => Promise<any>

// expected: (arg: string) => Promise<number>
// actual:   (arg: string) => Promise<number> ✔
const arg1UnknownError = promisify((arg: string, cb: (err: unknown, result: number) => void): void => { });

// expected: (arg: string) => Promise<number>
// actual:   (arg: string) => Promise<number> ✔
const arg1AnyError = promisify((arg: string, cb: (err: any, result: number) => void): void => { });

// expected: () => Promise<number>
// actual:   () => Promise<number> ✔
const arg0 = promisify((cb: (err: Error | null, result: number) => void): void => { });

// expected: () => Promise<any>
// actual:   () => Promise<void> --> I'd argue this is correct
const arg0NoResult = promisify((cb: (err: Error | null) => void): void => { });

// expected: (arg: string) => Promise<number>
// actual:   (arg: string) => Promise<number> ✔
const arg1 = promisify((arg: string, cb: (err: Error | null, result: number) => void): void => { });

// expected: (arg: string) => Promise<number>
// actual:   (arg: string) => Promise<number> ✔
const arg1UnknownError2 = promisify((arg: string, cb: (err: Error | null, result: number) => void): void => { });

// expected: (arg: string) => Promise<any>
// actual:   (arg: string) => Promise<void> --> I'd argue this is correct
const arg1NoResult = promisify((arg: string, cb: (err: Error | null) => void): void => { });

// expected: () => Promise<void | {}>
// actual:   () => Promise<void> --> where should the {} come from?
const cbOptionalError = promisify((cb: (err?: Error | null) => void): void => { cb(); }); // tslint:disable-line void-return

[DanielRosenwasser]

Well those seem way better. I don't know how I feel about those helper types, but if you wanted to put up your own PR to @types/node that might be a worthwhile way to gauge any other breaks.

[clonardo]

is there a way to switch back to the old behavior via a compilerOptions flag or something? TypeScript 4.1.2 is now shown on npm as a stable release, and this is a breaking change in my application.