tc39 / proposal-async-iteration

Asynchronous iteration for JavaScript
https://tc39.github.io/proposal-async-iteration/
MIT License
858 stars 44 forks source link

Manual using generator with `yield` without `await` #117

Closed Gvozd closed 6 years ago

Gvozd commented 6 years ago

I try code below in babel-node, and Chrome Canary(v64), and get different result Unfortunately I could not understand what behavior is correct by this spec

In this example I call iterator.next() twice, and synchronously 1) I expect that point#2 and task#2 will be called immediately(within second iterator.next()), because there is no await for the task#1 I give this behavior at babel-node, but not in Chrome Canary Chrome execute point#2 only when task#1 resolved, like as I wrote yield await task('task#1') instead yield task('task#1') 2) Should the promiseCapability/IteratorResult be resolved only when the result is resolved, and contain resolved value? 2.1) In babel-node IteratorResult resolved, when yield got the value(pending Promise), and return this value without additional awaiting. In other word IteratorResult..value can be a Promise 2.2) In Chrome IteratorResult resolved, when yield got the value(pending Promise), and this value also resolved. In other word IteratorResult..value cannot be a Promise - it is always fulfilled value

I read "Async Generator Rewrite", and it turns out that the behavior of the babel is correct But when I read https://github.com/tc39/proposal-async-iteration/issues/114, I'm confused

What is correct behavior for this example?

const start = Date.now(),
    iterator = iteratorBar();
logIteratorResult('next#1', iterator.next());
logIteratorResult('next#2', iterator.next());

async function* iteratorBar() {
    console.log(time(), 'point#1');
    yield task('task#1');
    console.log(time(), 'point#2');
    yield task('task#2');
}

async function task(value) {
    console.log(time(), `${value} - started`);
    return new Promise((resolve) => setTimeout(resolve, 1000, value));
}

async function logIteratorResult(name, iteratorResult) {
    let {value, done} = await iteratorResult;
    console.log(time(), `${name} IteratorResult - resolved`, value);
    value = await value;
    console.log(time(), `${name} IteratorResult.value - resolved`, value);
}

function time() {
    return Date.now() - start;
}

babel-node execution log

1 'point#1'
3 'task#1 - started'
4 'point#2'
4 'task#2 - started'
34 'next#1 IteratorResult - resolved' Promise { <pending> }
34 'next#2 IteratorResult - resolved' Promise { <pending> }
1004 'next#1 IteratorResult.value - resolved' 'task#1'
1004 'next#2 IteratorResult.value - resolved' 'task#2'

Chrome Canary execution log

1 "point#1"
2 "task#1 - started"
1002 "point#2"
1002 "task#2 - started"
1003 "next#1 IteratorResult - resolved" "task#1"
1003 "next#1 IteratorResult.value - resolved" "task#1"
2003 "next#2 IteratorResult - resolved" "task#2"
2004 "next#2 IteratorResult.value - resolved" "task#2"
gskachkov commented 6 years ago

@Gvozd Hmm, Safari Technical Preview the same result as in Chrome Canary.

As I know yield does always await in async generators

https://tc39.github.io/proposal-async-iteration/#sec-asyncgeneratoryield 11.4.3.7.8.b Also discussion https://github.com/tc39/tc39-notes/blob/master/es8/2017-05/may-25.md#15iva-revisiting-async-generator-yield-behavior

Conclusion/Resolution Go with option 3: yield has the semantics of yield await in an async generator

Gvozd commented 6 years ago

@gskachkov Thanks I read discusion and slides, and it became clear to me

It seems that Async Generator Rewrite is not entirely actual. And Chrome and Safari correctly supported discusion resolution.

Also after read slide 27, I got additional code with problem behavior @gskachkov, please tell what result you have in Safari

(async () => {
    const iterator = {
        i: 0,
        next() {
            if (this.i >= 3) {
                return {done: true};
            }
            return {
                value: Promise.resolve(this.i++),
                done: false
            };
        },
        [Symbol.asyncIterator]() {
            return this;
        }
    };

    for await(const i of iterator) {
        console.log(i);
    }
})();

babel-node execution log - not correct for slide 27

0
1
2

Chrome Canary log - correct for slide 27

Promise {<resolved>: 0}
Promise {<resolved>: 1}
Promise {<resolved>: 2}
Jamesernator commented 6 years ago

@gvozd This has been fixed in Babel 7 (currently Beta) so Babel's implementation should be compliant with the other implementations now.

gskachkov commented 6 years ago

@Gvozd I'm receiving following results in Safari Technical Preview:

Promise {status: "resolved", result: 0}
Promise {status: "resolved", result: 1}
Promise {status: "resolved", result: 2}
Gvozd commented 6 years ago

@Jamesernator I check with @babel/core@7.0.0-beta.32 My first example works correct, like Chrome/Safari But second example work not like Chrome/Safari, and for-await resolve my manual promise result

Also I was surprised, that transform-async-to-generator moved from stage-3, and disabled by preset-env I use node v9.2.0, that support async functions, but not for-await loop, and my node need only for-await transform(if possible), or both transformation

.babelrc

{
  "presets": [
    ["@babel/env", {
      "targets": {
        "node": "current"
      },
      "include": [
        // without this `for-await` not worked !!!
        "transform-async-to-generator"
      ]
    }],
    "@babel/stage-0"
  ]
}
gskachkov commented 6 years ago

async iterators is stil experimental feature in v8, so if you need fully support you can switch on it by

node --harmony-async-iteration

But it is not related to the current repo, it is better to close this issue.

caitp commented 6 years ago

pedantic correction :) that feature is shipped in v8 for some time, though Node.js hasn't updated to a version where it is shipped yet (It is staged since Node.js 9 and can be enabled with just --harmony)

RangerMauve commented 6 years ago

@caitp Good to know! I thought I had to wait for node to use 6.3 before I could get at this goodness!

Gvozd commented 6 years ago

@gskachkov Thanks for your help I got the answers I needed

domenic commented 6 years ago

Let me remove AsyncGeneratorRewrite, since it is causing confusion. Glad this all worked out though.

Gvozd commented 6 years ago

@domenic AsyncGeneratorRewrite.md is good idea - for me it was more understandable than spec I'll try to correct it for the actual behavior

domenic commented 6 years ago

Nah, it's OK, I'd rather not maintain two sources of truth in this repository.