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

Method overloading does not work on TSX templates #9703

Closed
drewwyatt opened this issue Jul 13, 2016 · 7 comments
Closed

Method overloading does not work on TSX templates #9703

drewwyatt opened this issue Jul 13, 2016 · 7 comments
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@drewwyatt
Copy link

drewwyatt commented Jul 13, 2016

TypeScript Version: 1.8.10
Code

import * as React from 'react';
import { Link } from 'react-router';

export interface ClickableProps {
    children?: string;
    className?: string;
}

export interface ButtonProps extends ClickableProps {
    onClick(event?: Event): void;
}

export interface LinkProps extends ClickableProps {
    to: string;
}

export function MainButton(buttonProps: ButtonProps): JSX.Element;
export function MainButton(linkProps: LinkProps): JSX.Element;
export function MainButton(props: ButtonProps | LinkProps): JSX.Element {
    const linkProps = props as LinkProps;
    if(linkProps.to) {
        return this._buildMainLink(props);
    }

    return this._buildMainButton(props);
}

function _buildMainButton({ onClick, children, className }: ButtonProps): JSX.Element {
    return(<button className={className} onClick={onClick}>{ children || 'MAIN BUTTON'}</button>);  
}

function buildMainLink({ to, children, className }: LinkProps): JSX.Element {
    return (<Link to={to}><button className={className}>{ children || 'MAIN BUTTON'}</button></Link>    );
}

/**
 * This works
 */

MainButton({ to: 'floop' }); // no errors
MainButton({ onClick: () => {} }); // no errors
MainButton(); // throws expected errors

/**
 * This does NOT work
 */

function buildSomeElement(): JSX.Element {
    return (
        <MainButton to='/some/path'>GO</MainButton>
    );
}

Expected behavior:

I would expect the buildSomeElement() function to not throw an error because the "to" attribute satisfies the second signature of the overloaded MainButton function (like what happens when this is called as a function and not used as a TSX element).

Actual behavior:

The buildSomeElement() function throws an error expecting an onClick attribute.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jul 13, 2016
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 13, 2016

Does this work if the function is declared like so (without the explicit signatures)?

export function MainButton(props: ButtonProps | LinkProps): JSX.Element {
    const linkProps = props as LinkProps;
    if(linkProps.to) {
        return this._buildMainLink(props);
    }

    return this._buildMainButton(props);
}
@drewwyatt
Copy link
Author

drewwyatt commented Jul 16, 2016

@RyanCavanaugh it does not. :( - that throws:

Property 'to' does not exist on type 'IntrinsicAttributes & (ButtonType | LinkType)'
@mynameisstephen
Copy link

It looks like only the first overloaded method is being evaluated. Moving

export function MainButton(linkProps: LinkProps): JSX.Element;

above

export function MainButton(buttonProps: ButtonProps): JSX.Element;

works as expected.

@RyanCavanaugh
Copy link
Member

This is actually rather complex but we're going to look at some targeted fix (mostly this example specifically and top-level union types in non-SFCs) in 2.1

@drewwyatt
Copy link
Author

Awesome!

@mhegazy mhegazy modified the milestones: TypeScript 2.1, TypeScript 2.1.2, Future Oct 27, 2016
@mhegazy mhegazy modified the milestones: TypeScript 2.2, Future Dec 23, 2016
@faceyspacey
Copy link

+1

@yuit yuit added the Fixed A PR has been merged for this issue label Jan 26, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.2, TypeScript 2.3 Feb 2, 2017
@mhegazy mhegazy removed this from the TypeScript 2.2 milestone Feb 2, 2017
@yuit
Copy link
Contributor

yuit commented Feb 15, 2017

@faceyspacey @drewwyatt the fix is now in master....we will now use similar overload resolution logic to figure out SFC. Give it a try and let us know!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
6 participants