Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer union type parameters instead of function overloads #46925

Closed
rveerd opened this issue Nov 26, 2021 · 2 comments
Closed

Prefer union type parameters instead of function overloads #46925

rveerd opened this issue Nov 26, 2021 · 2 comments
Labels
Duplicate An existing issue was already created

Comments

@rveerd
Copy link

rveerd commented Nov 26, 2021

lib Update Request

The type definitions for String.replace and String.replaceAll are written using overloads. Should they not be written with union type parameters?

Overloads give problems when creating wrapper functions for String.replace; see example below.

This has already been proposed in this comment in another issue but I cannot find a follow-up issue or discussion.

The documentation describes a similar problem and also advises to prefer union types, when possible.

Sample Code

export class Foo {
  constructor(
    private name: string
  ) { }

  replace(searchValue: RegExp | string, replaceValueOrReplacer: string | ((substring: string, ...args: any[]) => string)): string {
    return this.name.replace(searchValue, replaceValueOrReplacer);
  }
}
src/stream.ts:7:43 - error TS2769: No overload matches this call.
  The last overload gave the following error.
    Argument of type 'string | ((substring: string, ...args: any[]) => string)' is not assignable to parameter of type '(substring: string, ...args: any[]) => string'.
      Type 'string' is not assignable to type '(substring: string, ...args: any[]) => string'.

7     return this.name.replace(searchValue, replaceValueOrReplacer);
                                            ~~~~~~~~~~~~~~~~~~~~~~

  node_modules/typescript/lib/lib.es5.d.ts:454:5
    454     replace(searchValue: string | RegExp, replacer: (substring: string, ...args: any[]) => string): string;
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    The last overload is declared here.
@IllusionMH
Copy link
Contributor

IllusionMH commented Nov 26, 2021

Looks like duplicate of #44919.

And related request that should resolve it in general case #17471

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Nov 29, 2021
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants