feat(async): all commands have async versions#387
Conversation
| retValue = fn.apply(this, args); | ||
| if (config.async || callback) { | ||
| process.nextTick(function() { | ||
| var r = fn.apply(this, args); |
There was a problem hiding this comment.
This won't really work, because if the function itself blocks (ie. it uses blocking APIs), then this will too.
There was a problem hiding this comment.
Why would this not work? The function won't be called until everything on the event loop is dealt with, right?
There was a problem hiding this comment.
Yes, but imagine this scenario:
setTimeout(() => console.log('It\'s been 1 second!'), 1000);
exec('sleep 10; echo "It\'s been 10 seconds!"', function noop() {}, { async: true });Outputs:
It's been 10 seconds.
It's been 1 second.
There was a problem hiding this comment.
@ariporad I just tried this example. I saw:
It's been 1 second!
It's been 10 seconds!
Also, exec()'s async version is totally different than the rest of the commands.
Another example:
> setTimeout(() => console.log('It\'s been 1 second!'), 5000);
> ls({async: true}, function() { echo('hi'); });
Output (as I would expect, since ls() is super fast) :
hi
It's been 1 second!
There was a problem hiding this comment.
@nfischer: Whoops, my bad.
Try this (I know it's unrealistic times, but for argument's sake):
function sleep(time) {
var start = Date.now()
while (start + time > Date.now()) ;
};
setTimeout(() => console.log('1 sec'), 1000);
sleep(2000);
console.log('2 sec');Expected:
1 sec
2 sec
Actual:
2 sec
1 sec
There was a problem hiding this comment.
@ariporad your last example uses no shelljs. the actual output is what a JS programmer ought expect: you have to yield execution.
Fixes #2. This is an initial stab at async-ing everything. not ready to merge, since docs haven't been updated.
I think this is a nice interface, since you can specify commands to be async either with:
This is consistent with the precedent set by
exec().An alternative is to pass in the actual return value (usually a ShellString). This is better for using with pipes. I think the following is a suitable workaround, however, with this implementation: