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

[bug] yield* with .return() on parent, missed child's finally block execution #275

Closed
Andarist opened this issue Feb 1, 2017 · 4 comments
Assignees
Labels

Comments

@Andarist
Copy link
Contributor

Andarist commented Feb 1, 2017

The following code does not transform as expected:

function* saga(){
    try {
        yield* worker()
    } finally {
        console.log('parent > exit')
    }
}

function* worker(){
    try {
        yield 1 // just stop here
    } finally {
        yield 2
        console.log('child > exit')
    }
}

var t = saga()
t.next() // meet the stoping point
t.return()
t.next()

If we run this code in Chrome's console we'll get the following logs:

console.log('child > exit')
console.log('parent > exit')

but with regenerator the outcome is only this

console.log('parent > exit')

I would be happy to help with fixing this, but at least pointing some starting points in the codebase would be great, as it is not the easiest project to grasp at once

@benjamn
Copy link
Collaborator

benjamn commented Feb 17, 2017

This bug inspired me to spend some quality time with the 14.4.13 Generator Function Definitions: Runtime Semantics: Evaluation section of the specification, specifically the part labeled

YieldExpression : yield * AssignmentExpression

Although this bug could have been fixed without significantly rewriting the code that handles yield*, I'll sleep better knowing that I've followed the specification exactly.

Thanks for reporting this!

@Andarist
Copy link
Contributor Author

Wow, thanks! Just great work - although I was happy to help with the code I see I wouldnt have a chance to done this right, complex stuff in the commit fixing this!

What's need to be done so this fix is included in the next versions of babel?

@benjamn
Copy link
Collaborator

benjamn commented Feb 18, 2017

It's been a while since I thought about this code, so it wasn't easy for me either!

The babel-runtime package currently depends on regenerator-runtime@^0.10.0, which is compatible with 0.10.3, so I think these changes should be the default for new installs already?

For what it's worth, I did run the Babel test suite with this version installed, and saw no problems.

@Andarist
Copy link
Contributor Author

Yeah, u are right - new installs are fine! :) Thanks once again.

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

No branches or pull requests

2 participants