Skip to content

Overload conflicts make implementation impossible #4786

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
omidkrad opened this issue Sep 14, 2015 · 5 comments
Closed

Overload conflicts make implementation impossible #4786

omidkrad opened this issue Sep 14, 2015 · 5 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@omidkrad
Copy link

I created type definitions for Aurelia Event Aggregator as below. It compiles fine.

interface Constructor<T> {
    new (...args: any[]): T;
}

declare class EventAggregator
{
    subscribe(event: string, callback: (message: any) => void): void;
    subscribe<T>(event: Constructor<T>, callback: (message: T) => void): void;

    publish(event: string, data?: any): void;
    publish<T>(event: T): void;
}

class SomeMessage {
    constructor(public data: string) {}
}

var ea = new EventAggregator();

// event is string, message is any
ea.subscribe("myEvent", message => {
    console.log(message.anything);
});

// event is class constructor, message is instance of class
ea.subscribe(SomeMessage, message => {
    console.log(message.data);
});

// event is string, message is any 
ea.publish("myEvent", { anything: 1 });

// event is class instance
ea.publish(new SomeMessage(""));

Even though it is possible to define declarations for this class, it is impossible to implement it in TypeScript. The following produces errors, and I can't figure how to fix it.

interface Constructor<T> {
    new (...args: any[]): T;
}

class EventAggregator
{
    subscribe(event: string, callback: (message: any) => void): void;
    subscribe<T>(event: Constructor<T>, callback: (message: T) => void): void {
    }

    publish(event: string, data?: any): void;
    publish<T>(event: T): void {
    }
}

class SomeMessage {
    constructor(public data: string) {}
}

var ea = new EventAggregator();

// event is string, message is any
ea.subscribe("myEvent", message => {
    console.log(message.anything);
});

// event is class constructor, message is instance of class
ea.subscribe(SomeMessage, message => {
    console.log(message.data);
});

// event is string, message is any 
ea.publish("myEvent", { anything: 1 });

// event is class instance
ea.publish(new SomeMessage(""));

Is this a bug?

Here's the original JS source (ES6) for reference: aurelia/event-aggregator

@omidkrad omidkrad changed the title Overload conflicts Overload conflicts make implementation impossible Sep 14, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Sep 14, 2015

method definition is not counted as part of the overload set. you need to 1. define an overload signature of each overload, and 2. have an implementation signature that is compatible with all the overloads.

class EventAggregator {
    subscribe(event: string, callback: (message: any) => void): void;
    subscribe<T>(event: Constructor<T>, callback: (message: T) => void): void;
    subscribe(event: string|Constructor<any>, callback: (message: any) => void): void {

    }

    publish(event: string, data?: any): void;
    publish<T>(event: T): void;
    publish(event:any): void {
    }
}

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Sep 14, 2015
@danquirk
Copy link
Member

This is a common source of confusion. Can we improve the error here? If the assignability check fails could we then check assignability against the implementation signature to include more helpful error text (or have we already thrown away that context by then)? That might help people solve the issue without having to go from the IDE to StackOverflow/GitHub/etc.

@omidkrad
Copy link
Author

@mhegazy that worked. Thanks.

Is there any way to say that a parameter can take only object instances and not class constructors, so that for example ea.publish(SomeMessage) is not allowed?

@danquirk
Copy link
Member

A hacky option:

interface NotFunction {
    length?: void;
}

class EventAggregator {
    publish(event: string, data?: any): void;
    publish<T extends NotFunction>(event: T): void;
    publish(event:any): void { }
}

var e: EventAggregator;
e.publish(EventAggregator); // error
e.publish(1); // ok
e.publish(''); // ok
e.publish(new EventAggregator()); // ok

@omidkrad
Copy link
Author

Thank you very much. I'm closing this question. You may want to open a separate issue for improving the error message for this case.

@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