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

Yields are ignored in finally block when async generator delegation is used #45400

Closed
apendua opened this issue Aug 10, 2021 · 1 comment · Fixed by #51274
Closed

Yields are ignored in finally block when async generator delegation is used #45400

apendua opened this issue Aug 10, 2021 · 1 comment · Fixed by #51274
Assignees
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@apendua
Copy link
Contributor

apendua commented Aug 10, 2021

Bug Report

🔎 Search Terms

async
generator 
yield
finally

🕗 Version & Regression Information

Version: Version 4.3.5

  • This is the behavior in every version I tried (including nightly), and there doesn't seem to be any FAQ entries about this particular topic.

⏯ Playground Link

Playground link with relevant code

💻 Code

async function* generator1() {
  try {
    yield 1;
  } finally {
    yield 2;
  }
}

async function* generator2() {
  yield* generator1();
}

const it = generator2();

(async function () {
  console.log(await it.next());
  console.log(await it.return());
  console.log(await it.next());
}());

🙁 Actual behavior

The output of the above program is:

{ done: false, value: 1 }
{ done: true, value: undefined }
{ done: true, value: undefined }

This behavior can be observed for any compilation target lower than es2018. For es2018 and above there's no code transformation at all, and so the native implementation of async generators is used upon execution. This suggests that something is wrong with the corresponding polyfill. Indeed, after a closer examination:

var __asyncDelegator = (this && this.__asyncDelegator) || function (o) {
    var i, p;
    return i = {}, verb("next"), verb("throw", function (e) { throw e; }), verb("return"), i[Symbol.iterator] = function () { return this; }, i;
    function verb(n, f) { i[n] = o[n] ? function (v) { return (p = !p) ? { value: __await(o[n](v)), done: n === "return" } : f ? f(v) : v; } : f; }
};

seems to be ignoring the fact that calling it.return() does not necessarily result in { done: true }, which is effectively enforced by done: n === "return".

What is worth mentioning is the same problem doesn't occur for synchronous generators at all, which indicates that this is specific to how asynchronous generators are being handled.

🙂 Expected behavior

The output of the above program should be:

{ done: false, value: 1 }
{ done: false, value: 2 }
{ done: true, value: undefined }

which is in line with the behavior observed when native async generator implementation is used.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Aug 10, 2021
@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Dec 15, 2021
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Oct 22, 2022
@apendua
Copy link
Contributor Author

apendua commented Nov 12, 2022

@RyanCavanaugh Do you think there's a chance that the fix proposed in #51274 will be included in the upcoming TS release? Unfortunately, this is blocking my team from proceeding with TS migration because we have some code that relies on unconventional yields.

apendua added a commit to apendua/tslib that referenced this issue Nov 15, 2022
apendua added a commit to apendua/TypeScript that referenced this issue Nov 15, 2022
rbuckton pushed a commit that referenced this issue Nov 15, 2022
* Fix asyncDelegator reporting done too early

* Add unit test for yields inside finally block

See #45400
rbuckton pushed a commit to microsoft/tslib that referenced this issue Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants