Incorrect method overload resolution for some union types (e.g. in String.prototype.replace)

[noomorph]

Bug Report

🔎 Search Terms

string, replace, overload, union types

🕗 Version & Regression Information

It looks like this bug exists in all versions of TypeScript, including nightly, and yes, I've checked this: https://github.com/Microsoft/TypeScript/wiki/FAQ#common-bugs-that-arent-bugs

⏯ Playground Link

https://www.typescriptlang.org/play?ts=4.4.0-beta#code/PTAEGMHsAcEsFMAmoDOAXATgQ1gcwBZqgBmGkAtqAMqawB2uAdNGWpGgJ7TyMbzQAbLOHio8dLGgCufAFwAoeAA9okDEU7dQAJX5CRGAGJ1QAXlAAKFFIBG6DPVyzUtBgBpQjL1gy4UzrDoOAG0AXQBKMwA+FwcGAG5FFTUNLlFdQWF4DDNYx1AAHx09LKM6RPliKTpwNFhIExR8SCkBRAB1NQBrCz5Mg2cM-Wzw53t8gG95UBnQPmkMEwByAAl4AQFIDwB3NTaAQiXeEpELYECONHxHYA8+4YxwxIBfeUrq2vqTXYwunxa6IgACr4SSdX4oXonbKDaGPMauXCgKazUCwYiWTTwSAY+6lMymcxLcYMJaRFGo2bzGTLNYbLagH4HI5407nIJXG53OFPaazZ6gdYoUQUylzeALWnrTY7PaIQ7HfrwM4XTkMW7ipWPRKo16vIA

💻 Code

// copied straight from String.prototype.replace signature:
export type ReplacerFn = (substring: string, ...args: any[]) => string;
export type Replacer = string | ReplacerFn;

function shouldWork(replacer: Replacer): string {
    return 'Hello, world!'.replace(/anything/, replacer);
}

function workaroundThatWorks(replacer: Replacer): string {
    if (typeof replacer === 'string') {
        return 'Hello, world!'.replace(/anything/, replacer);
    } else {
        return 'Hello, world!'.replace(/anything/, replacer);
    }
}

🙁 Actual behavior

TypeScript prints a compilation error for the contents of shouldWork function:

No overload matches this call.
  The last overload gave the following error.
    Argument of type 'Replacer' is not assignable to parameter of type '(substring: string, ...args: any[]) => string'.
      Type 'string' is not assignable to type '(substring: string, ...args: any[]) => string'.(2769)
lib.es5.d.ts(454, 5): The last overload is declared here.

🙂 Expected behavior

I expect that the compiler would recognize that my argument of a custom union type Replacer will always fit one of the existing overloads since String.prototype.replace has them both:

interface String {
//...
    replace(searchValue: { [Symbol.replace](string: string, replaceValue: string): string; }, replaceValue: string): string;
    replace(searchValue: { [Symbol.replace](string: string, replacer: (substring: string, ...args: any[]) => string): string; }, replacer: (substring: string, ...args: any[]) => string): string;
}

[MartinJohns]

See #40827 and linked issues. A union type does not work for overload resolution.

[ilogico]

The overload resolution is correct, but maybe the replace method shouldn't be overloaded at all.
A single signature accepting a union as a second argument would be more flexible, and it wouldn't be a breaking change as far as I can tell.

[noomorph]

@MartinJohns, could the idea of @ilogico become a solution to my specific case, i.e., edit lib.es5.d.ts String.prototype.replace signature, or is it likely to break someone's else code?

[MartinJohns]

That decision would be up to the TypeScript team. I can't think of a reason why this would be a breaking change, but the documentation would definitely suffer from it.

[noomorph]

Okay, so I'll summarize what I've understood so far. This issue is a duplicate, and it belongs to a wontfix category. The official recommendation was given by Ryan Cavanaugh back in 2016:

…we discussed and our plan is to mitigate this by providing a TS Lint rule and guidance that you should never write a series of overloads that have an equivalent representation in union types.

Therefore, the only current way to fix this issue would be to convert replace signature to a less legible but union-type-compliant version, yet there is hesitation whether is the game worth a candle or not.

[fatcerberus]

there is hesitation whether is the game worth a candle or not

what

[RyanCavanaugh]

there is hesitation whether is the game worth a candle or not

what

I thought I'd heard every idiom but apparently not.

Anyway I'm tagging this for discussion because I think we should take another crack at union argument / overload resolution. The 24-combination-limited logic we added for property unions was Good and it seems like we could reuse that approach here to make the vast majority of cases work.