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

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

Closed
Gvozd opened this Issue Nov 20, 2017 · 12 comments

Comments

Projects
None yet
6 participants
@Gvozd
Copy link

Gvozd commented Nov 20, 2017

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 #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

This comment has been minimized.

Copy link

gskachkov commented Nov 20, 2017

@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

This comment has been minimized.

Copy link
Author

Gvozd commented Nov 22, 2017

@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

This comment has been minimized.

Copy link

Jamesernator commented Nov 22, 2017

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

@gskachkov

This comment has been minimized.

Copy link

gskachkov commented Nov 23, 2017

@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

This comment has been minimized.

Copy link
Author

Gvozd commented Nov 23, 2017

@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

This comment has been minimized.

Copy link

gskachkov commented Nov 23, 2017

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

This comment has been minimized.

Copy link

caitp commented Nov 23, 2017

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

This comment has been minimized.

Copy link

RangerMauve commented Nov 23, 2017

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

@Gvozd

This comment has been minimized.

Copy link
Author

Gvozd commented Nov 23, 2017

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

@Gvozd Gvozd closed this Nov 23, 2017

@domenic

This comment has been minimized.

Copy link
Member

domenic commented Dec 4, 2017

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

domenic added a commit that referenced this issue Dec 4, 2017

Remove "Async Generator Rewrite" document
Its out-of-dateness was causing confusion; see #117 for an example.
@Gvozd

This comment has been minimized.

Copy link
Author

Gvozd commented Dec 4, 2017

@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

This comment has been minimized.

Copy link
Member

domenic commented Dec 4, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.