Skip to content

Promise.catch should have a return type of <TResult>, not T. #3834

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
MicahZoltu opened this issue Jul 12, 2015 · 6 comments
Closed

Promise.catch should have a return type of <TResult>, not T. #3834

MicahZoltu opened this issue Jul 12, 2015 · 6 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@MicahZoltu
Copy link
Contributor

Current implementaiton:

interface Promise<T> {
    then<TResult>(onfulfilled?: (value: T) => TResult | PromiseLike<TResult>, onrejected?: (reason: any) => TResult | PromiseLike<TResult>): Promise<TResult>;
    then<TResult>(onfulfilled?: (value: T) => TResult | PromiseLike<TResult>, onrejected?: (reason: any) => void): Promise<TResult>;

    catch(onrejected?: (reason: any) => T | PromiseLike<T>): Promise<T>;
}

What I believe is a more correct implementation:

interface Promise<T> {
    then<TResult>(onfulfilled?: (value: T) => TResult | PromiseLike<TResult>, onrejected?: (reason: any) => TResult | PromiseLike<TResult>): Promise<TResult>;
    then<TResult>(onfulfilled?: (value: T) => TResult | PromiseLike<TResult>, onrejected?: (reason: any) => void): Promise<TResult>;

    catch<TResult>(onrejected?: (reason: any) => TResult | PromiseLike<TResult>): Promise<TResult>;
    catch<TResult>(onrejected?: (reason: any) => void): Promise<TResult>;
}

I believe the spec says that catch(x) behaves the same as then(undefined, x), which implies that catch should be a generic method with a TResult type.

If I am misunderstanding the spec, enlightenment would be appreciated.

@basarat
Copy link
Contributor

basarat commented Jul 13, 2015

catch<TResult>(onrejected?: (reason: any) => void): Promise<TResult>; is wrong in that if you have a catch and it does indeed end up getting called then if onRejected returns void the following promise chain will get Promise<void> not Promise<TResult>

That said the promise definition is best effort and not designed to be 100% correct, but the change you are recommending is not one that should be done 🌹

@basarat
Copy link
Contributor

basarat commented Jul 13, 2015

On second reading your suggestion isn't bad and maybe worth doing. I just wanted to point out that it is still best effort.

@danquirk
Copy link
Member

@rbuckton might have something to say here

@MicahZoltu
Copy link
Contributor Author

I'm actually not entirely sure what happens if you chain a promise to a catch that doesn't return. Does the original promise result get chained through? Does the chain get stopped prematurely?

Example:

promise.catch(error => {
    // intentionally does not return, is this considered a programming error?
}).catch(error => {
    console.log(error); // what does this do?
});

I believe the answer to this question would shed some light on what the signature should look like.

@basarat
Copy link
Contributor

basarat commented Jul 14, 2015

I believe the answer to this question would shed some light on what the signature should look like.

Here is a concrete example with comments to show what will happen:

Promise.reject(new Error('I want to get caught')).catch(error => {
    // I have caught the error and swallowed it silently
}).catch(error => {
    // This never gets called as the error has been caught by the previous catch
}).then(foo=>{
   // foo is the return of the first `catch` and therefore `void`
});

I just wanted to point out that it is still best effort.

Basically a true implementation starts to feel like JAVAs throws : http://stackoverflow.com/a/29287864/390330

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Feb 19, 2016
@RyanCavanaugh RyanCavanaugh added Declined The issue was declined as something which matches the TypeScript vision and removed In Discussion Not yet reached consensus labels Jun 23, 2021
@RyanCavanaugh
Copy link
Member

There doesn't seem to be much demand for this, and without sample code it's hard to evaluate what the deficit is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants