Skip to content

Make overload resolution work correctly with string literal types #10523

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

Closed
RyanCavanaugh opened this issue Aug 24, 2016 · 5 comments
Closed
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Aug 24, 2016

Our overload resolution with string literal types does not behave well. Today, we both issue incorrect errors due to selecting a "first" overload, and fail to catch errors due to falling back to a less-specific overload.

let type: any = "div";
// Error, can't convert HTMLAnchorElement to HTMLDivElement.
//  Where the heck did HTMLAnchorElement come from?
let elem1 = <HTMLDivElement>document.createElement(type);

let elem2 = document.createElement("div");
// Why is this not an error? Totally wrong event type specified.
elem2.addEventListener("click", (ev: ClipboardEvent) => {
    console.log(ev.clipboardData);
});

Typically we say "You should put your most general overload last", which is fine, except when invoking a function with any, you really want the most general overload to get selected. Instead we end up selecting the first overload, which is essentially random here (HTMLAnchorElement being alphabetically first is not a good reason to select it vs anything else).

I don't have any comprehensive fix here but we're wrong in both directions here and should look at what can be done.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Aug 24, 2016
@kitsonk
Copy link
Contributor

kitsonk commented Aug 24, 2016

Typically we say "You should put your most general overload last", which is fine, except when invoking a function with any

Well and wouldn't it be fair to say, when using an interface that is split up, putting most general last is/can be a challenge, leaving it up to "chance" as well?

@RyanCavanaugh
Copy link
Member Author

Let's not bring interface signature merging order into this 😰 . Anything we do here should play nicely with that existing behavior.

@felixfbecker
Copy link
Contributor

Another use case:

class X extends EventEmitter {
  on(event: 'myEvent', listener: (myParam: string) => any);
}

@geoffreak
Copy link

To consolidate my duplicate issue, here's another use case presenting the problem:

async function addEvent(eventType: '$login', email: string | undefined, data: LoginEvent): Promise<void | Error>;
async function addEvent(eventType: '$logout', email: string, data?: LogoutEvent): Promise<void | Error>;
async function addEvent(eventType: string, email: string | undefined, data: Object = {}): Promise<void | Error> {
  // ...
}

interface SiftScienceBaseEvent {
  $ip?: string;
  $time?: number;
}
interface LoginEvent extends SiftScienceBaseEvent {
  $session_id: string;
  $login_status: '$success' | '$failure';
}
interface LogoutEvent extends SiftScienceBaseEvent {
  // This page intentionally left blank
}

let sid: string | undefined;
addEvent('$login', undefined, {
  $login_status: '$success',
  $session_id: sid,
});

The error I expected to see: (and you can indeed see by commenting out the $logout overload)

Types of property $session_id are incompatible

The error I see instead:

Argument of type '"$login"' is not assignable to parameter of type '"$logout"'

@RyanCavanaugh
Copy link
Member Author

Seems like this problem is something we can live with by writing overloads more smartly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants