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

Overloading functions with specialized parameters not working as expected. #6746

Closed
seshualluvada opened this issue Jan 30, 2016 · 4 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@seshualluvada
Copy link

The below code has a specialized signature for Subscribe function that takes "NotificationClicked" as the event Name and NotificationClickedEventArgs type as the eventArgs parameter. The general signature takes any string as eventName and the base EventArgs as the eventArgs parameter.

My understanding is that typescript should give a compiler error where I am executing the code PublishSubscribe.Subscribe("NotificationClicked", new EventArgs()); because the specialized signature expects a NotificationClickedEventArgs type. As per my observation, typescript is compiling successfully which most likely is a bug. This works correctly on return types though.

class EventArgs{
  EventName: string;
  EventData: any;
}

class NotificationClickedEventArgs extends EventArgs{
  NotificationID: string;
}

class PublishSubscribe{
  static Subscribe:{
    (eventName: "NotificationClicked", eventArgs: NotificationClickedEventArgs):void;
    (eventName: string, eventArgs :EventArgs):void;
  } = (eventName: string, eventArgs: EventArgs) => {  }
}

PublishSubscribe.Subscribe("NotificationClicked", new EventArgs());
@DanielRosenwasser
Copy link
Member

This is because you have that catch-all non-specialized signature. Unfortunately, you can't remove it because of an old requirement before string literal types became a thing. #6278 seeks to fix that, but I I'm not sure if we can get it into 1.8. @mhegazy @ahejlsberg?

@seshualluvada
Copy link
Author

This behavior is working as expected with return type overloading as shown below. Even though I have the non-specialized signature that is returning the base EventArgs type, TypeScript is deducing accurately that the Subscribe method will be returning NotificationClickedEventArgs when the parameter passed is "NotificationClicked", which makes me believe that the type checking for the following parameter types is not getting stricter based on the parameter values provided for previous parameters.

class EventArgs{
  EventName: string;
  EventData: any;
}

class NotificationClickedEventArgs extends EventArgs{
  NotificationID: string;
}

class PublishSubscribe{
  static Subscribe:{
    (eventName: "NotificationClicked"):NotificationClickedEventArgs;
    (eventName: string):EventArgs;
  } = (eventName: string):EventArgs => { 
        if (eventName === "NotificationClicked")
          return new NotificationClickedEventArgs();  
        else
          return new EventArgs();
      }
}

PublishSubscribe.Subscribe("NotificationClicked").NotificationID //This is allowed
PublishSubscribe.Subscribe("SomethingElse").NotificationID //This errors as expected as NotificationID is not a property of EventArgs

@mhegazy
Copy link
Contributor

mhegazy commented Feb 19, 2016

With #6744 in place (you can try it in typescript@next), you can remove the string overload, and that will give you the error you expect.

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Feb 19, 2016
@mhegazy mhegazy closed this as completed Feb 19, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Feb 19, 2016

also worth noting, is it will make your function more restrictive, i.e.

class PublishSubscribe{
  static Subscribe:{
    (eventName: "NotificationClicked", eventArgs: NotificationClickedEventArgs):void;
  };
}

PublishSubscribe.Subscribe("NotificationClicked", new EventArgs()); // Erro EventArg is not NotificationClickedEventArgs

PublishSubscribe.Subscribe("NotificationClicked", new NotificationClickedEventArgs()); //  OK

var eventName = "NotificationClicked";
PublishSubscribe.Subscribe(eventName, new EventArgs()); // Error, string is not "NotificationClicked"

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

3 participants