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

Generics: Function's template param inferred in unexpected way #20643

Closed
UselessPickles opened this issue Dec 12, 2017 · 9 comments
Closed

Generics: Function's template param inferred in unexpected way #20643

UselessPickles opened this issue Dec 12, 2017 · 9 comments
Labels
Duplicate An existing issue was already created

Comments

@UselessPickles
Copy link

TypeScript Version: 2.6.2

Code

// visitor interface for visiting possible values of a string literal type
type StringVisitor<S extends string> = {
    [P in S]: (value: P) => void;
}

function visitString<S extends string>(value: S, visitor: StringVisitor<S>): void {
    visitor[value](value);
}


// visitor interface for visiting possible values of a string literal type, or null/undefined
type NullableStringVisitor<S extends string> = {
    handleNullOrUndefined: (value: null | undefined) => void;
} & StringVisitor<S>

function visitNullableString<S extends string>(
    value: S | null | undefined, 
    visitor: NullableStringVisitor<S>
): void {
    if (value == null) {
        visitor.handleNullOrUndefined(value);
    } else {
        visitString(value, visitor);
    }
}

// example string literal type
type RGB = "red" | "green" | "blue";

function getRGBValue(): RGB | null {
    return "green";
}

// example shared handler used for multiple values in the RGB visitor implementation
function handleUnsupportedValue(value: RGB | null | undefined): void {
    alert('Unsupported value: ${value}');
}

visitNullableString(getRGBValue(), {
    // Compiler error caused here:
    // Types of property 'handleNullOrUndefined' are incompatible.
    // Type '(value: "red" | "green" | "blue" | null | undefined) => void' is not assignable 
    // to type '(value: "handleNullOrUndefined") => void'
    handleNullOrUndefined: handleUnsupportedValue,

    "red": handleUnsupportedValue,

    "green": () => {
        alert("green!");
    },

    "blue": () => {
        alert("blue!");
    }
});

Expected behavior:
In the call to visitNullableString, type S should be directly inferred from param value as "red" | "green" | "blue", and the expected type of handleNullOrUndefined on visitor should be (value: null | undefined) => void, because visitor should be of type NullableStringVisitor<"red" | "green" | "blue">.

Actual behavior:
Type S is indirectly inferred from param visitor as "red" | "green" | "blue" | "handleNullOrUndefined", based on the names of all properties in the visitor implementation. The compiler then complains about incompatible types on handleNullOrUndefined, because the implementation does not accept the string literal type "handleNullOrUndefined".

Workaround
If I explicitly provide the template param, then it works as desired:

visitNullableString<RGB>(getRGBValue(), {
    // No compiler error
    handleNullOrUndefined: handleUnsupportedValue,

    "red": handleUnsupportedValue,

    "green": () => {
        alert("green!");
    },

    "blue": () => {
        alert("blue!");
    }
});
@UselessPickles UselessPickles changed the title Generics: Template param inferred in unexpected way Generics: Function's template param inferred in unexpected way Dec 12, 2017
@RyanCavanaugh
Copy link
Member

This could be solved by either subtraction types (#4183), or noninferential type parameters (#14829). As is, this is the intended behavior because the intersection type is a lower-priority inference site than the object literal's.

@UselessPickles
Copy link
Author

@RyanCavanaugh Could you provide an example of how I would use type subtraction to get the behavior I want? This is my initial guess:

function visitNullableString<S extends (string - "handleNullOrUndefined")>(
    value: S | null | undefined, 
    visitor: NullableStringVisitor<S>
): void

The non-inferred type parameter solution is more obvious:

function visitNullableString<S extends string>(
    value: S | null | undefined, 
    visitor: NullableStringVisitor<NoInfer<S>>
): void

@UselessPickles
Copy link
Author

SOLUTION!!! (hack???)

type NoInfer<T> = T & {[K in keyof T]: T[K]};

function visitNullableString<S extends string>(
    value: S | null | undefined, 
    visitor: NullableStringVisitor<NoInfer<S>>
): void

Thanks to this comment for the NoInfer<T> implementation: #14829 (comment)

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Dec 13, 2017
@UselessPickles
Copy link
Author

Follow up on the previous hack: it's good enough to give me something functional for now, but it would be much more useful if I could prevent inferring the type of visitor altogether, and instead force the type of visitor to be determined entirely based on the inferred type S from the value param:

function visitNullableString<S extends string>(
    value: S | null | undefined, 
    visitor: NoInfer<NullableStringVisitor<S>>
): void

However, the hack NoInfer<T> implementation from my previous comment is very limited in that it can only prevent inference on a particular usage of a template param that is used more than once in inferrable ways (in this case, type S).

This limitation is preventing me from overloading a single visitString method that handles different combinations of value types (S, S | null, S | undefined, S | null | undefined), and appropriately corresponding variations of StringVisitor types that include handler methods for null and/or undefined, based on whether the type of the value param includes null and/or undefined.

Technically, I can overload the method as desired, and it works when correct params are passed in. But when incorrect params are passed in (visitor missing a handler for one of the string values, or for null/undefined), then the compiler error messages are very misleading because it ends up inferring the type of the visitor incorrectly, completely independent of the type of value, telling the developer that their visitor is missing a property that is unnecessary based on the type of value.

function visitString<S extends string>(
    value: S, 
    visitor: StringVisitor<NoInfer<S>>
): void;
function visitString<S extends string>(
    value: S | null | undefined, 
    // When a call to visitString is made with incompatible types for 'value' and visitor',
    // the compiler seems to always default to assuming this last overloaded signature
    // which can cause an error to complain about missing"handleNullOrUndefined"
    // property even when the value of `value` passed in can never be null/undefined
    visitor: NullableStringVisitor<NoInfer<S>> 
): void;

For now, the best I can reasonably do is a separate method for each combination of null/undefined. The result is helpful error messages when a visitor is not fully implemented with all necessary handlers, but the developer must explicitly choose the correct variation of visitString (e.g., visitStringWithNull vs visitStringWithNullOrUndefined, etc.) to match the type of the value they pass in.

Here's a full example in the Playground with separate visit methods (enable ALL compiler options). Let me know if you'd also like me to setup an example with my attempt to overload the visitString method, showing how the compiler errors are misleading compared to my intentions.

@UselessPickles
Copy link
Author

Or maybe my new issue is more about function overloading rather than type param inference?

@UselessPickles
Copy link
Author

UselessPickles commented Dec 14, 2017

Nevermind on my previous long comment. I've figured out that it's an issue with compile error reporting on overloaded functions. Creating a new issue... #20697

@UselessPickles
Copy link
Author

My final solution is to split things up so the compiler only has to deal with inferring/enforcing the type of one parameter at a time.

My original goal was to have a single overloaded method. Example:

function visitString<S extends string, R>(value: S, visitor: StringVisitor<S, R>): R;
function visitString<S extends string, R>(value: S | null, visitor: StringVisitorWithNull<S, R>): R;

The goal being that if a potentially null value is passed in, then a corresponding appropriate visitor that can handle null must be supplied.

This approach had 3 distinct issues:

In my final solution, the signature(s) of my method looks more like this:

declare function visitString<S extends string>(value: S, visitor): StringVisitee<S> ;
declare function visitString<S extends string>(value: S | null): StringVisiteeWithNull<S>;

And the StringVisitee classes are simple wrappers around the value to be visited, and provides a generic method that actually accepts the visitor:

declare class StringVisitee<S extends string> {
    public with<R>(visitor: StringVisitor<S, R>): R;
}
declare class StringVisiteeWithNull<S extends string> {
    public with<R>(visitor: StringVisitorWithNull<S, R>): R;
}

And the complete usage now looks like:

declare x: RGB;

visitString(x).with({
    "red": () => { alert("RED!"); },
    "green": () => { alert("GREEN!"); },
    "blue": () => { alert("BLUE!"); },
});

This solves all 3 problems I had:

  • The visitString method now only has to infer the type of the value being visited (specifically, whether it can be null or not), which dictates the kind of StringVisitee returned, which in turn accepts only the correct kind of visitor to its with method - no inference on the visitor any more (except for the return type portion of it).
  • If you pass a mismatch of params to visitString and with, it is now very clear that the type of value passed to visitString drives the type of visitor expected by the with method, so you get very clear error messages about what's wrong with your visitor. The visitor is now strictly enforced to have the handleNull method if, and only if, the type of value may be null.
  • The with method has only one type param: the return type. So you can now explicitly provide the desired return type to help enforce correct implementations.

Example of explicit return type:

declare x: RGB;

const result = visitString(x).with<number>({
    "red": () => { 
        return 1; 
    },
    "green": () => { 
        return 2; 
    },
    "blue": () => { 
        // very clear compiler error that this method returns the wrong type
        return "WRONG!";
    },
});

I implemented a full example in the playground (must enable strictNullChecks!)

@UselessPickles
Copy link
Author

In case anyone comes across this and is interested in this string visitor pattern I've been trying to create, I have now fully documented and tested it and put it on github: https://github.com/UselessPickles/ts-string-visitor

And published it as an npm package: https://www.npmjs.com/package/ts-string-visitor

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants