[Bug]: Regression in for await transform, is not awaiting iterator values

[Jessidhia]

💻

• Would you like to work on a fix?

How are you using Babel?

babel-loader (webpack)

Input code

Minimal example that shows the compilation issue:

async function f() {
  for await (const x of [].push()) {}
}

More elaborate example that crashes at runtime if f is invoked:

async function f() {
  for await (const { foo: { bar } } of [1].map(async x => ({ foo: { bar: x } }))) {}
}

Configuration file name

babel.config.js

Configuration

https://babeljs.io/repl defaults with not ie 11 in "targets" changed to ie 11

Current and expected behavior

(This section was a red herring as the codegen change was intended, the actual issue is that the AsyncFromSyncIterator behavior was not updated to match the new codegen; see #13811 (comment))

Babel is miscompiling the sample for await loop in such a way that it's effectively building a regular for of loop on top of regenerator. While the iterator.next() call is being awaited, the yielded value is not itself getting awaited as it's supposed to, so the looped over values are Promise instances instead of their values.

This is a regression from (at least) 7.13. The REPL doesn't seem to have 7.14 so I couldn't check that.

Relevant annotated output in 7.13.17:

        case 4:
          _context.next = 6;
          // await iterator.next()
          return regeneratorRuntime.awrap(_iterator.next());

        case 6:
          _step = _context.sent;
          // store whether iterator is complete but don't check yet to not leak the potential Promise 
          _iteratorNormalCompletion = _step.done;
          _context.next = 10;
          // await step.value (what was yielded from iterator.next())
          return regeneratorRuntime.awrap(_step.value);

        case 10:
          // read the result of await step.value
          _value = _context.sent;

          // check now for iterator completion
          if (_iteratorNormalCompletion) {
            _context.next = 16;
            break;
          }

          // set the loop variable to the awaited iterator.value
          x = _value;

Relevant annotated output in 7.15.7:

        case 4:
          _context.next = 6;
          // await iterator.next()
          return regeneratorRuntime.awrap(_iterator.next());

        case 6:
          // directly check if iterator is done before awaiting for value
          if (!(_iteratorAbruptCompletion = !(_step = _context.sent).done)) {
            _context.next = 11;
            break;
          }

          // directly uses step.value, which at this point is a Promise instead of the yielded value
          x = _step.value;
          // Promise is leaked if user doesn't redundantly await the value inside the loop

Environment

  System:
    OS: Linux 5.10 Ubuntu 20.04.3 LTS (Focal Fossa)
  Binaries:
    Node: 16.5.0 - ~/.nodenv/versions/16.5.0/bin/node
    Yarn: 3.0.0 - ~/.nodenv/versions/16.5.0/bin/yarn
    npm: 7.19.1 - ~/.nodenv/versions/16.5.0/bin/npm
  Monorepos:
    Yarn Workspaces: 3.0.0
  npmPackages:
    @babel/core: ^7.15.5 => 7.15.5
    @babel/runtime: ^7.15.4 => 7.15.4
    jest: ^27.0.6 => 27.0.6

Possible solution

No response

Additional context

No response

[lightmare]

While the iterator.next() call is being awaited, the yielded value is not itself getting awaited as it's supposed to, so the looped over values are Promise instances instead of their values.

The yielded value is not supposed to be awaited, though:

(step 6.f.) https://tc39.es/ecma262/multipage/ecmascript-language-statements-and-declarations.html#sec-runtime-semantics-forin-div-ofbodyevaluation-lhs-stmt-iterator-lhskind-labelset

https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-asynciterator-interface

[Jessidhia]

Oh, right, you are correct; the codegen here is not necessarily wrong. The code I expected is, however, part of the AsyncFromSyncIterator behavior (it awaits value before returning iterator result), and it doesn't seem to be happening either. I'll check again once I'm not on the phone.

[Jessidhia]

Yep, checking the full codegen:

// reformatted with prettier for readability
function _asyncIterator(iterable) {
  var method
  if (typeof Symbol !== 'undefined') {
    if (Symbol.asyncIterator) method = iterable[Symbol.asyncIterator]
    if (method == null && Symbol.iterator) method = iterable[Symbol.iterator]
  }
  if (method == null) method = iterable['@@asyncIterator']
  if (method == null) method = iterable['@@iterator']
  if (method == null) throw new TypeError('Object is not async iterable')
  return method.call(iterable)
}


function f() {
  var _iteratorAbruptCompletion, _didIteratorError, _iteratorError, _iterator, _step, x;

  return regeneratorRuntime.async(function f$(_context) {
    while (1) {
      switch (_context.prev = _context.next) {
        case 0:
          _iteratorAbruptCompletion = false;
          _didIteratorError = false;
          _context.prev = 2;
          _iterator = _asyncIterator([].push());

        case 4:
          _context.next = 6;
          return regeneratorRuntime.awrap(_iterator.next());

        case 6:
          if (!(_iteratorAbruptCompletion = !(_step = _context.sent).done)) {
            _context.next = 11;
            break;
          }

          x = _step.value;

// ...

The asyncIterator helper just directly returns the normal @@iterator if there's no @@asyncIterator, without actually wrapping it in an AsyncFromSyncIterator. The AsyncFromSyncIterator is supposed to unwrap the value before producing the final result (note 9 in the algorithm explicitly spells it out).

The old codegen happened to guard against asyncIterator's implementation but it no longer does.

[Jessidhia]

Looks like the codegen change was done in #13491 (with no helper change)

[lightmare]

Thanks for the detailed explanation. So I guess another way to show the bug would be:

async function* bar() {
  yield* [Promise.resolve("ok")] // CreateAsyncFromSyncIterator
}

bar().next().then(console.log);
// should print { value: 'ok', done: false }