Method overloads are often frustrating for me

TypeScript Version: 4.0.x
Search Terms: overload

Code

enum SideBySideEditor {
    PRIMARY,
    SECONDARY,
    BOTH
}

interface URI { }

interface IEditorInput { }

interface IResourceOptions {
    supportSideBySide?: SideBySideEditor;
}

function toResource(editor: IEditorInput | undefined | null): URI | undefined;
function toResource(editor: IEditorInput | undefined | null, options: IResourceOptions & { supportSideBySide?: SideBySideEditor.PRIMARY | SideBySideEditor.SECONDARY }): URI | undefined;
function toResource(editor: IEditorInput | undefined | null, options: IResourceOptions & { supportSideBySide: SideBySideEditor.BOTH }): URI | { primary?: URI, secondary?: URI } | undefined;
function toResource(editor: IEditorInput | undefined | null, options?: IResourceOptions): URI | { primary?: URI, secondary?: URI } | undefined {
    // Method implementation left out for brevity.
    return undefined;
}

export function getOriginalUri(editor: IEditorInput | undefined | null): URI | undefined;
export function getOriginalUri(editor: IEditorInput | undefined | null, options: IResourceOptions & { supportSideBySide?: SideBySideEditor.PRIMARY | SideBySideEditor.SECONDARY }): URI | undefined;
export function getOriginalUri(editor: IEditorInput | undefined | null, options: IResourceOptions & { supportSideBySide: SideBySideEditor.BOTH }): URI | { primary?: URI, secondary?: URI } | undefined;
export function getOriginalUri(editor: IEditorInput | undefined | null, options?: IResourceOptions): URI | { primary?: URI, secondary?: URI } | undefined {

    // What I would like to type:
    // return toResource(editor, options);

    // What I have to type
    if (!editor) {
        return undefined;
    }

    if (!options) {
        return toResource(editor);
    }

    if (options.supportSideBySide === SideBySideEditor.BOTH) {
        return toResource(editor, options);
    }

    if (options.supportSideBySide === SideBySideEditor.PRIMARY || options.supportSideBySide === SideBySideEditor.SECONDARY) {
        return toResource(editor, options);
    }
}

Expected behavior:
I can call the one method with overloads from the other without having to do a lot of checks.

Actual behavior:
I am often running into a problem when dealing with methods that define different overloads based on the types passed in. This works great if the method is called from the outside, but if internally I split the method up into 2 (as in the example above) I fail to call the one from the other somehow. I need to either duplicate all the code or do a lot of type checks in the second method that I will have to do again in the first method.

If there is guidelines how to do this any better, that would also help me.

Playground Link: Playground

Duplicate of #39885 / #1805.

I think you should really have a third overload on toResource:

function toResource(editor: IEditorInput | undefined | null): URI | undefined;
function toResource(editor: IEditorInput | undefined | null, options: IResourceOptions & { supportSideBySide?: SideBySideEditor.PRIMARY | SideBySideEditor.SECONDARY }): URI | undefined;
function toResource(editor: IEditorInput | undefined | null, options?: IResourceOptions & { supportSideBySide: SideBySideEditor.BOTH }): URI | { primary?: URI, secondary?: URI } | undefined;
function toResource(editor: IEditorInput | undefined | null, options?: IResourceOptions): URI | { primary?: URI, secondary?: URI } | URI | undefined;
function toResource(editor: IEditorInput | undefined | null, options?: IResourceOptions): URI | { primary?: URI, secondary?: URI } | undefined {
    return undefined;
}

export function getOriginalUri(editor: IEditorInput | undefined | null): URI | undefined;
export function getOriginalUri(editor: IEditorInput | undefined | null, options: IResourceOptions & { supportSideBySide?: SideBySideEditor.PRIMARY | SideBySideEditor.SECONDARY }): URI | undefined;
export function getOriginalUri(editor: IEditorInput | undefined | null, options: IResourceOptions & { supportSideBySide: SideBySideEditor.BOTH }): URI | { primary?: URI, secondary?: URI } | undefined;
export function getOriginalUri(editor: IEditorInput | undefined | null, options?: IResourceOptions): URI | { primary?: URI, secondary?: URI } | undefined {
    // OK
    return toResource(editor, options);
}

The thing to remember is that the implementation signature isn't visible (this is intentional) - in many cases (but not all, which is why this is intentional), you do need to write that signature "twice" (once as an overload, once as implementation). Without that last signature, this API design makes very little sense IMO, which is also why TS has a hard time reasoning about it.

Ryan Cavanaugh (@RyanCavanaugh)

Ryan Cavanaugh (@RyanCavanaugh) thanks, I learned something. Was not aware that the implementation signature is hidden.

Back to #4797. 😢

Ryan Cavanaugh (@RyanCavanaugh)

Here's an especially frustrating and simple example of what I believe to be the same issue, which just happened to me:

findAll(path: string, createIfMissing: true): Node[];
findAll(path: string, createIfMissing: false): Node[] | null;
findAll(path: string, createIfMissing: boolean): Node[] | null {/* ... */}

find(path: string, createIfMissing: true): Node;
find(path: string, createIfMissing: false): Node | null;
find(path: string, createIfMissing: boolean): Node | null {
    const nodes = this.findAll(path, createIfMissing);
    //                               ^^^^^^^^^^^^^^^ Error (see below)
    if (!nodes) return null;
    return nodes[nodes.length - 1];
}

The error displayed on createIfMissing:

No overload matches this call.
  Overload 1 of 2, '(path: string, createIfNone: true): Node<D>[]', gave the following error.
    Argument of type 'boolean' is not assignable to parameter of type 'true'.
  Overload 2 of 2, '(path: string, createIfNone: false): Node<D>[] | null', gave the following error.
    Argument of type 'boolean' is not assignable to parameter of type 'false'. ts(2769)

What makes it ridiculous is that if the createIfMissing has to be either true or false, then why should I be unable to pass it into a function that can take in either of those values?

Thanks to Ryan Cavanaugh (@RyanCavanaugh)'s comment above mentioning that the implementation signature is hidden, I actually managed to come up with a workaround for this specific issue while explaining it above, so here it is in case anyone else has the same issue:

Instead of declaring two specific overloads, in this instance you can simply declare one specific and one more general overload, and TypeScript should use the more specific one when possible. In practice:

findAll(path: string, createIfMissing: true): Node[];
findAll(path: string, createIfMissing: boolean): Node[] | null;
findAll(path: string, createIfMissing: boolean): Node[] | null {/* ... */}

// Same for `find()`

Additionally, if you want your implementation to have a default value, you will need to explicitly mark the argument as optional in the overload, or TypeScript will require that all calls provide all arguments:

findAll(path: string, createIfMissing: true): Node[];
findAll(path: string, createIfMissing?: boolean): Node[] | null;
findAll(path: string, createIfMissing = false): Node[] | null {/* ... */}

Navigation Menu

Repository navigation

Method overloads are often frustrating for me #40827

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Method overloads are often frustrating for me #40827

Description

Activity

MartinJohns commented on Sep 29, 2020

RyanCavanaugh commented on Sep 29, 2020

bpasero commented on Sep 29, 2020

DanielRosenwasser commented on Oct 1, 2020

opl- commented on Oct 5, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions