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

finally is now stage 2 #35

Closed
JakeChampion opened this issue Aug 25, 2016 · 14 comments
Closed

finally is now stage 2 #35

JakeChampion opened this issue Aug 25, 2016 · 14 comments

Comments

@JakeChampion
Copy link
Contributor

When do we consider adding finally to yaku?
https://github.com/tc39/proposal-promise-finally

From yaku readme:

  • Will Yaku implement done, finally, etc?

No. All non-ES6 APIs are only implemented for debugging and testing, which means when you remove Yaku, everything should work well with ES6 native promise.

@ysmood
Copy link
Owner

ysmood commented Aug 27, 2016

Thanks, I'll implement it ASAP.

@ysmood
Copy link
Owner

ysmood commented Aug 29, 2016

ES6 is locked, so it should be a part of ES7 or somehow. I'm sorry that I'm a little outdated.

So I don't know, where to implement it, as an extension or inside the core. I need more information.
I did some research on the internet, nothing seems helpful. I even can't figure out it's ES7 or ES-Next.

@JakeChampion
Copy link
Contributor Author

It's not in ES2016/ES7, only two Math methods were added to ES2016/ES7. It will hopefully enter Stage 3 at some point this year.

Criteria to move to Stage 3:

  • Complete spec text
  • Designated reviewers have signed off on the current spec text
  • The ECMAScript editor has signed off on the current spec text

It is up to you where to implement it. I imagine people would be confused if they had to install Yaku and a Yaku plugin to get ES-Promises working.

@ysmood
Copy link
Owner

ysmood commented Aug 29, 2016

Uh, it should be inside the core.

ysmood added a commit that referenced this issue Sep 1, 2016
ysmood added a commit that referenced this issue Sep 1, 2016
ysmood added a commit that referenced this issue Sep 1, 2016
@JakeChampion
Copy link
Contributor Author

@ysmood Do you need some help with this feature at all?

@ysmood
Copy link
Owner

ysmood commented Nov 1, 2016

Yes, I need some help. The performance is my main concern.

@ysmood
Copy link
Owner

ysmood commented Nov 1, 2016

The two extra closure makes me headache. I may merge the finally branch if I have time to add more test cases.

ysmood added a commit that referenced this issue Dec 9, 2016
@ysmood
Copy link
Owner

ysmood commented Dec 9, 2016

@JakeChampion I published the version v0.17.0 with the newly added finally support. Hope its not too late.

@ysmood ysmood closed this as completed Dec 9, 2016
@JakeChampion
Copy link
Contributor Author

Thanks!

@JakeChampion
Copy link
Contributor Author

Looks to fail some tests in these browsers:

 • ie/11
 • ie/10
 • ie/9
 • ie/8
 • android/4.4

@ysmood
Copy link
Owner

ysmood commented Dec 12, 2016

May I see the error information?

@JakeChampion
Copy link
Contributor Author

Rerunning the test suite now as I had some configuration errors which reported lots of false positives. Apologies if it turns out everything is working.

@JakeChampion
Copy link
Contributor Author

Tests pass on all except IE8 which seems to hang

@ysmood
Copy link
Owner

ysmood commented Dec 13, 2016

I will double check it later.

This issue was closed.
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

2 participants