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

Incorrect overload is used for composed function inside pipe #25637

Closed
OliverJAsh opened this issue Jul 13, 2018 · 19 comments
Closed

Incorrect overload is used for composed function inside pipe #25637

OliverJAsh opened this issue Jul 13, 2018 · 19 comments
Labels
Duplicate An existing issue was already created

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Jul 13, 2018

TypeScript Version: 2.9.2

Search Terms: generic function pipe contextual type wrong overload

Code

These examples are stripped down versions of Node's parse function in the querystring module and Lodash's values and fromPairs functions.

These are issues we ran into when trying to use these functions with a pipe function.

declare function pipe<A, B, C>(ab: (a: A) => B, bc: (b: B) => C): (a: A) => C;

{
    type ParsedUrlQuery = { [index: string]: string };
    declare function parsedUrlQueryString(str: string): ParsedUrlQuery;
    declare function parsedUrlQueryString<T extends {}>(str: string): T;

    declare const stringToString: (s: string) => string;

    // $ExpectType (a: string) => ParsedUrlQuery
    // but got (a: string) => {}
    // Note: if we comment out the last overload for `parsedUrlQueryString`, we get the expected
    // type.
    const fn = pipe(
        stringToString,
        parsedUrlQueryString,
    );
}

{
    declare function values<T>(object: { [index: string]: T } | null | undefined): T[];
    declare function values<T extends object>(object: T | null | undefined): Array<T[keyof T]>;
    declare function values(object: any): any[];

    declare const stringDictToStringDict: (
        sd: { [index: string]: string },
    ) => { [index: string]: string };

    // $ExpectType (a: { [index: string]: string; }) => string[]
    // but got (a: { [index: string]: string; }) => any[]
    // Note: if we comment out the last two overloads for `values`, we get the expected type.
    const fn = pipe(
        stringDictToStringDict,
        values,
    );
}

{
    declare function fromPairs<T>(pairs: [string, T][] | null | undefined): { [index: string]: T };
    declare function fromPairs(pairs: any[][] | null | undefined): { [index: string]: any };

    // $ExpectType (a: {}) => { [index: string]: number; }
    // but got (a: {}) => { [index: string]: any; }
    // Note: if we comment out the last overload for `fromPairs`, we get the expected type.
    const fn = pipe(
        ({  }: {}): [string, number][] => [['foo', 1]],
        fromPairs,
    );
}
@dardino
Copy link

dardino commented Jul 13, 2018

Hi, I have a similar problem:

Playground

function a(value: any, replacer?: (string | number)[] | null, space?: string | number): string;
function a(value: any, replacer?: ((k: string, v: any) => any) | null, space?: string | number): string;
function a(value: any, replacer?: ((k: string, v: any) => any) | (string | number)[] | null, space?: string | number): string {
    return "";
}

var b = (value: any, replacer?: ((k: string, v: any) => any) | (string|number)[] | null, space?: string | number): string => {
    if (replacer instanceof Array || typeof replacer === "function")
        return a(value, replacer, space); // Why ERROR ??? function "a" supports both Array and function as 2nd parameter
}

@OliverJAsh
Copy link
Contributor Author

Hey @mhegazy, any idea if this is a bug or design limitation? Hopefully my examples demonstrate the frequency of this issue.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 19, 2018

In @dardino's example the overloads do not include the implementation signature, so there is no overload that accepts (string | number)[] | ((k: string, v: any) => any), only ones that accept (string | number)[] | null or ((k: string, v: any) => any) | null`.

@mhegazy
Copy link
Contributor

mhegazy commented Jul 19, 2018

In the OP:
first set of overloads, the second overload of parsedUrlQueryString seems meaningless. removing it should cause the compiler to infer the right type.
Second and third sets, not sure what is going on, need to debug to figure it out

@mhegazy mhegazy added the Needs Investigation This issue needs a team member to investigate its status. label Jul 19, 2018
@OliverJAsh
Copy link
Contributor Author

I simplified the parsedUrlQueryString overloads to demonstrate and isolate the issue—the full example does require that overload.

@dardino
Copy link

dardino commented Jul 20, 2018

In @dardino's example the overloads do not include the implementation signature, so there is no overload that accepts (string | number)[] | ((k: string, v: any) => any), only ones that accept (string | number)[] | null or((k: string, v: any) => any) | null`.

@mhegazy Sure, but i think this is a bit limiting thing.
in fact I can write this to compile correctly: (that is very heavy)

var b = (value: any, replacer?: ((k: string, v: any) => any) | (string|number)[] | null, space?: string | number): string => {
    if (replacer instanceof Array)
        return a(value, replacer, space);
    if (typeof replacer === "function")
        return a(value, replacer, space);
}

I believe that is possible to make this assumption:
If all available overloads of a method can be called with current parameters types because they are compatible then the compiler can accept the call without errors:

class C1 { P1: string; }
class C2 { P2: string; }
class C3 { P3: string; }

function a(value: C1 | null): string;
function a(value: C2 | null): string;
function a(value: C1 | C2 | null): string {
    return "";
}

var c = (value?: C1 | C2 | C3 | null | undefined): string => {
    if (value instanceof C1 || value instanceof C2)        
        return a(value); // C1 | C2  --> not null or undefined, there is no reasons to give an error...
        // is a(C1) a valid call? YES
        // is a(C2) a valid call? YES
        // why error? 
    return value ? value.P3 : "";
}

var b = (value?: C1 | C2 | C3 | null | undefined): string => {
    if (value instanceof C1) return a(value);  // C1
    if (value instanceof C2) return a(value);  // C2
    return value ? value.P3 : "";
}

@OliverJAsh
Copy link
Contributor Author

@dardino This issue is concerning overloads when used with a pipe function, which doesn't appear to be the case with your issue. Perhaps you could open a new issue to avoid conflating the issue being tracked here?

@dardino
Copy link

dardino commented Jul 21, 2018

ok

@OliverJAsh OliverJAsh changed the title Wrong overloads are used for generic functions inside pipe Wrong overloads are used for generic functions inside pipe Jul 23, 2018
@Erikvv
Copy link

Erikvv commented Sep 26, 2018

Looks like a duplicate of #10957

@weswigham
Copy link
Member

Ref DefinitelyTyped/DefinitelyTyped#28592 which seems to have been motivated by this.

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Feb 13, 2019

Revisiting this. This isn't specific to generic composed functions. I believe the issue can be narrowed down to:

{
    declare const pipe: {
        <A, B, C>(ab: (a: A) => B, bc: (b: B) => C): (a: A) => C;
    };

    declare const myFn: {
        (p: string): string;
        (p: any): number;
    };

    // Expected type: `(a: string) => string`
    // Actual type: `(a: string) => number`
    // Note: if we comment out the last overload for `myFn`, we get the expected
    // type.
    const fn = pipe(
        (s: string) => s,
        myFn,
    );
}

Any ideas @weswigham? Does this help?

@OliverJAsh OliverJAsh changed the title Wrong overloads are used for generic functions inside pipe Incorrect overload is used for generic functions inside pipe Feb 14, 2019
@OliverJAsh OliverJAsh changed the title Incorrect overload is used for generic functions inside pipe Incorrect overload is used for composed function pipe Feb 14, 2019
@OliverJAsh OliverJAsh changed the title Incorrect overload is used for composed function pipe Incorrect overload is used for composed function inside pipe Feb 14, 2019
@RyanCavanaugh
Copy link
Member

This is definitely #10957 - overloads aren't considered during generic inference

@RyanCavanaugh RyanCavanaugh added Duplicate An existing issue was already created and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 23, 2019
@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.

5 similar comments
@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.

@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.

@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.

@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.

@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.

@jack-williams
Copy link
Collaborator

You may rest now bot.

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

8 participants