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

Add support for Promise's .finally() method. #5810

Closed
zachgoldstein opened this issue Feb 13, 2018 · 5 comments
Closed

Add support for Promise's .finally() method. #5810

zachgoldstein opened this issue Feb 13, 2018 · 5 comments

Comments

@zachgoldstein
Copy link

zachgoldstein commented Feb 13, 2018

Using the promise method .finally() will break flow's typing. Here's a silly example:

 31:     let test = new Promise(() => {}).finally(() => console.log('test'))
                                          ^^^^^^^ property `finally`. Property not found in
 31:     let test = new Promise(() => {}).finally(() => console.log('test'))
                    ^^^^^^^^^^^^^^^^^^^^^ Promise

Is there a clean way to use the promise method .finally() (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/finally) and not break Flow's typing?

In core.js I see it's not supported on the Promise object: https://github.com/facebook/flow/blob/master/lib/core.js#L591-L610. Is there a particular reason why?

@ghost
Copy link

ghost commented Feb 14, 2018

Does Flow support proposal APIs? You could derive a new class from Promise until it lands in core.js:

declare class PromiseWithFinally<+R> extends Promise<R> {
  finally<U>(onFinally: () => U): Promise<U>;
}

@saadq
Copy link

saadq commented Feb 14, 2018

It seems like Promise.prototype.finally is stage 4. I think this means it is a "finished" proposal and should probably be included now in core.js.

@lll000111
Copy link
Contributor

lll000111 commented Feb 18, 2018

Not only is it stage 4, it already is available in the standard releases of both Firefox and Chrome, and Microsoft's ChakraCore has it in the code already (just merged into master 3 days ago).

@coaxial
Copy link

coaxial commented Apr 12, 2018

Ran into this as well, finally is part of the promise API now, Flow should see it as such

@dsainati1
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants