-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Overloads shouldn't differ by the return type of a function type. #571
Comments
Your first example gives an error because a function cannot return a I could understand that someone would like to write something like this: function foo(getValue: () => string);
function foo(getValue: () => Promise<string>);
function foo(getValue: () => any) {
var value = getValue();
if (value instanceof Promise) {
// ...
} else {
// ...
}
} Also changing the current behavior would be a breaking change, which should be avoided because it can break existing code. |
@ivogabe Thanks for replying. Though I have mentioned that I thought both of the behaviors are reasonable, but may not be good enough. Consider this simplified promise interface: interface IPromise<T> {
then<R>(onfulfilled: void, onrejected: (reason) => T): IPromise<T>;
then<R>(onfulfilled: (value: T) => R, onrejected?: (reason) => R): IPromise<R>;
}
var promise: IPromise<void>;
promise
.then(value => {
return '';
})
.then(null, reason => {
return 0;
})
.then(value => {
// value here gets type "number"
}); As what I've commented, The logic of compiler is simple (as how TypeScript compiler should be), the return value of the second As As for the question whether this would break current code, my opinion is that the impact could be considered minor. |
What exactly is the proposal here? We're not going to break the |
Hi @RyanCavanaugh , I do think it's useful. What I mean is that, we may separate overload matching into two phases. During phase one, match the parameter types. And phase two, check the return type. It might be several return types in an interface, it would be okay. However, if a function argument, such as a callback, matches the parameter types, but not the return type, the compiler should give an error instead of just skipping this match. |
Think about this from the invoker of the callback's perspective -- they are going to invoke the function with some predefined set of arguments, then do something with the return value. It's possible for the invoker to determine what they got back from the callback, but it's impossible for them to know what type of parameters the function was expecting before calling it. Having the compiler match on the parameter values first is backwards from how any implementation could work in reality. |
However by allowing multiple return types would mostly solve the backwards issue imo (And this is exactly what people need to do now if they want to allow multiple return types, too). |
Can you post a minimal example of what you're describing and how it would work? |
example 1 interface IHandler {
// allow multiple return types for the same parameter types.
// as it's an interface, it doesn't need to concern the distinguishing
// problem for developers.
// and so far this won't have any backwards issue I think.
(): string;
(): number;
}
var handler: IHandler = () => {
// either returning a string or number should be okay.
return '';
return 0;
// however if trying to return something else, the compiler should give an error.
return true;
return undefined;
// ...
}; example 2 interface IPromise<T> {
then<R>(onfulfilled: void, onrejected: (reason) => T): IPromise<T>;
then<R>(onfulfilled: (value: T) => R, onrejected?: (reason) => R): IPromise<R>;
}
var promise1: IPromise<string>;
var promise2 = promise1.then(null, reason => {
return 0;
});
// following current spec, it would:
// 1. try to match the first overload, then fail because onrejected handler
// asks type T (i.e. string here) for return value.
// 2. try to match the second overload, then succeed because it should.
// so there will not be an error, and promise2: IPromise<number>.
// for the proposed spec:
// I hope it would firstly match an overload by parameter types except function types,
// and parameter types of these function types if any.
// if the return types also matched that overload, everybody is happy.
// if not, try to find another overload that has exactly the same parameter types
// except function types, and parameter types of these function types if any, as the
// overload found before.
// if another matched overload is found, do return type checking. if it succeed,
// happy. if not, try process above again.
// if none completely matching overload, gives an error:
// if any matching overload could be found before checking return type, it gives
// an error of unmatched return type.
// if no matching overload could be found even before checking return type, it gives
// an error of no overload found.
var promise1: IPromise<string>;
var promise2 = promise1.then(null, reason => {
// so here it should give an error because the return type is not T (i.e. string) or
// anything else declared possible (though here only T).
return 0;
});
// promise2: IPromise<string>; |
Again, the proposed behavior doesn't correspond to any runtime semantics that could exist. We've tried many, many, many overload resolution rules and are satisfied with the ones we have unless there is some very good reason to change them (in a way that is not a breaking change). Some of the rules proposed in #360 might help with your scenario, depending. |
If we define an interface:
There would be error "overloads cannot differ only by return type", which is reasonable.
However, when it comes to some situation like this:
The compiler would treat
(str: string) => string
and(str: string) => number
as two incompatible types, which is also reasonable but not useful enough.I would suggest to use parameter types to defer and use return type to do type checking only.
The real situation could be
Promise<T>.prototype.then
, because it accepts null as parameter, if one of the two arguments is null, it will skip the signature we would want it to match and adopts to something wrong. We may add an overload withany
as the return type of its function type, but this would mute type checking.The text was updated successfully, but these errors were encountered: