Improve output of for await

[liuxingbaoyu]

Q	A
Fixed Issues?	Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/55411/

[nicolo-ribaudo]

This accesses .done on the object twice, which is observable if it is a getter.

[nicolo-ribaudo]

Can we do something like this?

  async function wrapper() {
    try {
      for (
        var ITERATOR_KEY = GET_ITERATOR(OBJECT), STEP_KEY, NOT_DONE_KEY;
        NOT_DONE_KEY = !(STEP_KEY = await ITERATOR_KEY.next()).done;
      ) {
      }
    } finally {
      try {
        if (NOT_DONE_KEY && ITERATOR_KEY.return != null) {
          await ITERATOR_KEY.return();
        }
      } catch (e) {}
    }
  }

[liuxingbaoyu]

We need to set NOT_DONE_KEY to false after the loop ends to avoid executing return() when it throws in next().

[nicolo-ribaudo]

Can you check if this test behaves properly:

function* gen() {
  try {
    yield 1;
  } finally {
    throw 2;
  }
}

let err;
try {
  for await (const _ of gen()) break;
} catch (e) {
  err = e;
}

expect(err).toBe(2);

[liuxingbaoyu]

Yes, this PR breaks the test.
After fixing it it only saved two variables. 🤦‍♂️
But luckily our testing is now better. :)

[nicolo-ribaudo]

We should submit that test to test262, since it didn't catch the bug!

[lightmare]

Is there an upside to initializing STEP_KEY here? It'll be overwritten before use.

[liuxingbaoyu]

Now it's no longer needed, thank you!

[nicolo-ribaudo]

Suggested change

	if (STEP_KEY) throw e;
	if (STEP_KEY != null) throw e;

.next() might set Boolean.prototype.done to true and return false :)

Could you also rename _notDone to _iteratorAbruptCompletion, to reduce the diff and make it easier to follow the changes?

[lightmare]

nit: .next() must return an Object; otherwise agree explicit null-check looks clearer.

ForIn/OfBodyEvaluation

6.c. If nextResult is not an Object, throw a TypeError exception.

[liuxingbaoyu]

.next() might set Boolean.prototype.done to true and return false :)

I'm not sure if it's worth doing, it's really weird usage. :)

Could you also rename _notDone to _iteratorAbruptCompletion, to reduce the diff and make it easier to follow the changes?

Since I completely changed the position of variable declarations and the logic of throwing exceptions, the changes may not be less, _notDone is slightly shorter and easier to read in the output.

[nicolo-ribaudo]

If calling y[Symbol.asyncIterator]() throws, does this still behave the same?

[nicolo-ribaudo]

If _iterator.return is falsy but not nullish, we should still call it to get the proper TypeError.

[lightmare]

Only undefined should skip the call.

[nicolo-ribaudo]

I have been thinking about this more, and I think we can avoid the catch block. Does this code lead to smaller minified size, and still passes all the tests?

const buildForAwait = template.statements(`
  var ITERATOR_KEY = GET_ITERATOR(OBJECT), STEP_KEY, NOT_DONE_KEY, ITERATOR_HAD_NO_ERROR_KEY;
  try {
    for (
      ;
      NOT_DONE_KEY = !(STEP_KEY = await ITERATOR_KEY.next()).done;
      NOT_DONE_KEY = false
    ) {
    }
    ITERATOR_HAD_NO_ERROR_KEY = true;
  } finally {
    try {
      if (NOT_DONE_KEY && ITERATOR_KEY.return) {
        await ITERATOR_KEY.return();
      }
    } catch (e) {
      if (!ITERATOR_HAD_NO_ERROR_KEY) throw e;
    }
  }
`);

[nicolo-ribaudo]

It might have problems if iterator.return() throws and the for-await-of body uses return 🤔

[lightmare]

It might have problems if iterator.return() throws and the for-await-of body uses return 🤔

Yes, to follow the spec, you need to know whether the loop body completed with

• a) throw => that exception propagates, and exception from iterator.return() is ignored
• b) return / break outer / continue outer => exception from iterator.return() propagates

And the only way to tell the difference, is to catch the exception.