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

Await inside for loop does not work #13

Closed
Rush opened this issue Sep 27, 2016 · 16 comments
Closed

Await inside for loop does not work #13

Rush opened this issue Sep 27, 2016 · 16 comments

Comments

@Rush
Copy link

Rush commented Sep 27, 2016

async liveBackup(files) {
  /* some other code here */
  for(let file of files) {
    await execFileAsync('touch', [backupFile(file.fullPath)]);
  }
}
{ TypeError: /code/app/lib/api/machines.js: Duplicate declaration "file" (This is an error on an internal node. Probably an internal error)
    at File.buildCodeFrameError (/code/app/node_modules/babel-core/lib/transformation/file/index.js:431:15)
    at Scope.checkBlockScopedCollisions (/code/app/node_modules/babel-traverse/lib/scope/index.js:397:27)
    at Scope.registerBinding (/code/app/node_modules/babel-traverse/lib/scope/index.js:579:16)
    at Scope.registerDeclaration (/code/app/node_modules/babel-traverse/lib/scope/index.js:483:14)
    at Object.BlockScoped (/code/app/node_modules/babel-traverse/lib/scope/index.js:185:28)

It works fine with babel's async-to-module-method.

@Rush
Copy link
Author

Rush commented Sep 28, 2016

This function is part of an object. I will try to make a small reproducible test-case.

@matAtWork
Copy link
Collaborator

Thanks. From the stack trace, it looks like it might be related to the let. If you can get something to work in the playground, but not in Babel, let me know as it might be an unpleasant interaction between Babel and Nodent.

@Rush
Copy link
Author

Rush commented Sep 28, 2016

@matAtWork ok, this the most minimal case that breaks:

async function liveBackup() {
  for(let file of files) {
    await Promise.resolve();
  }
  for(let file of files) {
    await Promise.resolve();
  }
}
npm install fast-async-issue-13
cd node_modules/fast-async-issue-13
npm test

@matAtWork
Copy link
Collaborator

Yep, that's a bug. Nodent is mangling the scope for declarations within a for-loop. I'll work on a fix. The only workaround I can suggest short-term is to use different variable names in your for loop declarations, or use 'var' since it has function scope.

@Rush
Copy link
Author

Rush commented Sep 28, 2016

I was merely curious about nodent. Using async to bluebird coroutine works fine. I will give it another shot once the bug is fixed though.

btw. will stacktraces show proper line numbers in nodent when combined with babel?

@matAtWork
Copy link
Collaborator

I doubt it. Nodent's stack trace mapping is a run-time feature, which I imagine Babel mangles as the sourcemaps generated by Nodent won't make through to execution. TBH, I've not tried - it might work depending on how transparent Babel is.

I've fixed the bug in Nodent v3 (not released yet), and almost back-ported it to v2.6.x but one of the main differences is how async loops are transpiled so it's a work in progress.

@Rush
Copy link
Author

Rush commented Sep 28, 2016

Nice, I'll try it out after the release. If you generate an inline source map then babel should pick it up.

@matAtWork
Copy link
Collaborator

Just published v2.6.10 which (I hope) fixes this issue. Please check on the link above.

I've not updated fast-async (yet) as I'll do that when I'm ready to ship v3. You should get the update automatically if you uninstall and re-install fast-async in your project.

Let me know how it goes, and thanks for the report

@Rush
Copy link
Author

Rush commented Sep 28, 2016

It seems to have fixed it. Also, sourcemaps seem to work but they are not enabled by default. Is it a bug?

I had to add the option to .babelrc

{
  "presets": [],
  "plugins": [["fast-async", {
    "compiler": {
      "sourcemap": true
    } 
  }]]
}
Error
    at $ForStatement_1_loop (/code/fast-async-issue-13/test.js:4:17)
    at Function.boundThen [as then] (/code/fast-async-issue-13/test.js:122:21)
    at /code/fast-async-issue-13/test.js:1:1
    at boundThen (/code/fast-async-issue-13/test.js:122:21)
    at liveBackup (/code/fast-async-issue-13/test.js:1:1)
    at Object.<anonymous> (/code/fast-async-issue-13/test.js:13:1)
    at Module._compile (module.js:556:32)
    at loader (/code/fast-async-issue-13/node_modules/babel-register/lib/node.js:143:5)
    at Object.require.extensions.(anonymous function) [as .js] (/code/fast-async-issue-13/node_modules/babel-register/lib/node.js:153:7)
    at Module.load (module.js:473:32)
Error
    at $ForStatement_1_loop (/code/fast-async-issue-13/test.js:4:17)
    at $ForStatement_1_next (/code/fast-async-issue-13/test.js:1:1)
    at /code/fast-async-issue-13/test.js:5:5
    at then (/code/fast-async-issue-13/test.js:115:126)
    at process._tickCallback (internal/process/next_tick.js:103:7)
    at Module.runMain (module.js:592:11)
    at run (bootstrap_node.js:394:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:509:3
Error
    at $ForStatement_1_loop (/code/fast-async-issue-13/test.js:4:17)
    at $ForStatement_1_next (/code/fast-async-issue-13/test.js:1:1)
    at /code/fast-async-issue-13/test.js:5:5
    at then (/code/fast-async-issue-13/test.js:115:126)
    at process._tickCallback (internal/process/next_tick.js:103:7)
    at Module.runMain (module.js:592:11)
    at run (bootstrap_node.js:394:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:509:3
foo
async function liveBackup() {
  let files = ['aaa', 'fff', 'ewrrewewrewr'];
  for(let file of files) {
    console.log(new Error().stack);
    await Promise.resolve();
  }
  for(let file of files) {
    await Promise.resolve();
  }
  return 'foo';
}

liveBackup().then(console.log);

@Rush
Copy link
Author

Rush commented Sep 28, 2016

This is really weird. After running my test after couple minutes, the sourcemaps stopped working and then on another run they worked fine (in my small test, anyway). There is something really quirky going on.

Anyway, the performance of your project is impressive but without sourcemaps this cannot be used in production. :-(

@Rush
Copy link
Author

Rush commented Sep 28, 2016

Anyway, I'm gonna close this issue and open another one.

@Rush Rush closed this as completed Sep 28, 2016
@jscinoz
Copy link

jscinoz commented Jan 22, 2018

This issue is still present in 2018 as of fast-async 6.3.0.

@jscinoz
Copy link

jscinoz commented Mar 5, 2018

It seems my issue is a bit different (same error, but to do with a name collision across distinct branches of an if statement where const is used within - perfectly valid and works correctly when fast-async isn't involved.

I suspect it's an interaction with another Babel plugin in our project - I'll comment back here when I've figured it out.

@jscinoz
Copy link

jscinoz commented Mar 5, 2018

Seems it was an issue with the ordering of Babel plugins in our project - having fast-async last fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants