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

Grammar for interfacing existing callback code with async/await #38

Closed
MatAtBread opened this Issue Apr 5, 2015 · 10 comments

Comments

Projects
None yet
4 participants
@MatAtBread

MatAtBread commented Apr 5, 2015

Whilst Promises provide an mechanism to relate the (considerable) body of callback-based code with async functions, the syntax is relatively verbose and implementation dependent (i.e. it requires and relies on knowledge of the spawn() function) potentially restricting the possible set of implementations.

This issue suggests a mechanism to minimize the boilerplate needed to interface existing code with async/await by allowing callbacks contained within async functions to resolve and reject their container async functions, and prevent the container resolving automatically on exit without a return or throw.

Consider the code below, which has only 1 functional line (2 including the declaration):

function sleep(t) {
    return new Promise(function(resolve,reject) {
        setTimeout(resolve,t) ;
    }) ;
}

An alternative would be to pass the resolve/reject parameters to the re-written generator with two additional parameters (in this example asyncReturn and asyncThrow), parsing

async function sleep(t) {
    setTimeout(asyncReturn,t) ;
}

to

function sleep(t) {
    return spawn(function*(asyncReturn,asyncError){
        setTimeout(asyncReturn,t) ;
    }) ;
}

The same translation works for direct calls as well:

async function get(url) {
    var x = new XMLHttpRequest() ;
    x.onload = function(){
        if (x.status===200)
            return asyncReturn(x.responseText) ;
        else
            return asyncError(new Error(x.responseText)) ;
    } ;
   x.open(url) ;
   x.send() ;
}

...translates to

function get(url) {
    return spawn(function*(async,asyncError){
      var x = new XMLHttpRequest() ;
      x.onload = function(){
          if (x.status===200)
              return asyncReturn(x.responseText) ;
          else
              return asyncError(new Error(x.responseText)) ;
      } ;
     x.open(url) ;
     x.send() ;
   })
}

This can be easily implemented by:

  1. Passing the Promise resolve and reject routines in spawn():
function spawn(genF,self) {
    return new Promise(function(resolve, reject) {
        var gen = genF.call(self,resolve,reject);
        ....
  1. Allowing async functions to NOT automatically resolve on return, but providing a method to indicate resolution will take place in the future. I have implemented this by disallowing the resolved value to be the resolver argument (since this is pointless), and always appending a synchronous return to the wrapped generator body.

The rewrite rule now becomes:

async function <name>?<argumentlist>{<body>}
=>
function <name>?<argumentlist>{ return spawn(
    function*(asyncReturn,asyncError) {<body> ; return asyncReturn ; },
this); }

...and spawn is updated to:

function spawn(genF,self) {
    return new Promise(function(resolve, reject) {
        var gen = genF.call(self,resolve,reject);
        function step(nextF) {
            var next;
            try {
                next = nextF();
            } catch(e) {
                // finished with failure, reject the promise
                reject(e); 
                return;
            }
            if(next.done) {
                // finished with success, resolve the promise
                // ...unless the response indicates resolution by callback
                if (next.value!==resolve)
                    resolve(next.value);
                return;
            } 
            // not finished, chain off the yielded promise and `step` again
            Promise.cast(next.value).then(function(v) {
                step(function() { return gen.next(v); });      
            }, function(e) {
                step(function() { return gen.throw(e); });
            });
        }
        step(function() { return gen.next(undefined); });
    });
}

Syntax:
The use of the identifier asyncReturn (and asyncError) is possibly not wise. Additional rewrites would be required, but preferential syntaxes might be:

return async <expr>     =>     return <arg1>(<expr>)
throw async <expr>      =>     return <arg2>(<expr>)
@getify

This comment has been minimized.

getify commented Apr 6, 2015

I understand the problem trying to be solved, but I would be -1 on adding additional keywords like asyncReturn and asyncThrow. I already feel like async function has stretched this additional function syntax to the limit (and possibly slightly past it).

@MatAtBread

This comment has been minimized.

MatAtBread commented Apr 6, 2015

I don't disagree at all about introducing new keywords - the use of 'asyncReturn' and 'asyncError' was to demonstrate one implementation.

In our own code I've re-used async to extend return and throw (as in the final comment in the original issue). You can see this in practice at http://nodent.mailed.me.uk/ - take a look at the webhttp.js example.

The use case is, in my mind, very common. We have quite a lot of code using async/await in production, much of it on top of callback based libraries running on node v0.10 and Chrome. The ability to have a platform/implementation independent mechanism for having a callback cause an async function to respond is very useful. The alternative is each developer assuming certain platform features or wrapping existing functions in one-or-another Promise implementation and testing heavily.

@domenic

This comment has been minimized.

Member

domenic commented Apr 6, 2015

The future is longer than the past. Promises are standardized and returned from future-facing APIs. Adapting legacy patterns is not something we should build in to the language.

@MatAtBread

This comment has been minimized.

MatAtBread commented Apr 6, 2015

That's pretty philosophical! I know we have different approaches to development, but this use case is borne out of a lot of effort getting async/await into production (pretty successful so far).

It might be a feature of our early adoption, but it will definitely be an issue faced by a lot of developers. I thought it might be worth sharing with the community as having a standardized way of interfacing "the past" to the future eased our path considerably.

I prefer to think of it less as "adapting legacy patterns" and more as "helping developers move into the future successfully".

@domenic

This comment has been minimized.

Member

domenic commented Apr 6, 2015

I'm all about helping developers move into the future successfully! I just don't think that's something the language itself should be responsible for.

@MatAtBread

This comment has been minimized.

MatAtBread commented Apr 6, 2015

Dominic - you're a very clever guy, and I'm a big fan of your blog and work, but I don't see how that moves things forward. For us mortals, language support for otherwise fiddly constructs (like making async/await different from yield/generators - you made a very good comment on why this is a good thing on https://esdiscuss.org/topic/does-async-await-solve-a-real-problem) is one way to help us. Indeed in that post, you made a convincing case for using the language to do something that could easily be done by a search and replace (although I think it's also a good move for VM providers who can optimize that case separately from the iteration case).

You may wish there was no need for this use case, but unfortunately there is. Since we're in the early stages of working out exactly what opportunities and difficulties async and await introduce, maybe it's worth taking on board everyone's experiences and seeing which common cases would benefit from language support?

@getify

This comment has been minimized.

getify commented Apr 6, 2015

It seems to me the concern is actually related to the fact that Promises don't have an external deferred -- a really old argument I don't think will ever be reopened.

return new Promise(function(resolve,reject) {
  setTimeout(resolve,t) ;
}) ;

could have been:

var d = new Deferred();
setTimeout(d.resolve,t);
return d.promise;

IOW, it seems to me like your complaint is needing to wrap the promise constructor function around a piece of code to do the extraction of the resolution capability. Without that requirement, there doesn't seem to be any need for extra syntax of any sort to fake this deep return behavior.


So, just fix that by making your own Deferred helper that does the capability extraction, as several promise libs have, like this:

function Deferred() {
   var def = {}, p = new Promise(function(resolve,reject){ def.resolve = resolve, def.reject = reject; });
   def.promise = p;
   return def;
}

Now:

async function sleep(t) {
   var d = Deferred();
   setTimeout(d.resolve,t) ;
   return d.promise;
}

Of course, when you're only talking about 1-line async functions, adding those two extra lines is unfortunate noise. But in practice, it seems most async functions will probably be less trivial, and thus a couple of quick lines shouldn't be too much of a tax for adapting legacy to the new world.

@MatAtBread

This comment has been minimized.

MatAtBread commented Apr 6, 2015

That's certainly a sensible approach. I'm not sure that the extra promise construction and resolution will perform as well as directly resolving the async implementation promise, but I guess it avoids additional syntax, even if it doesn't do such a good job of hiding the implementation details.

I'll work through some examples and see how it compares. Thanks for the input

@lukehoban

This comment has been minimized.

Contributor

lukehoban commented Apr 9, 2015

Agreed with @domenic and @getify that we shouldn't introduce new syntax for these cases. I'm not sure the original code is all that bad for the cases where you need to interface with callback code - especially given that there are so many callback styles in common use that this will have to be somewhat custom per-case anyway.

The Deferred approach may be more comfortable for some - though I find these two options similar in overall complexity:

function sleep(t) {
    return new Promise(resolve => 
        setTimeout(resolve,t)
    );
}
async function sleep(t) {
   var d = Deferred();
   setTimeout(d.resolve,t) ;
   return d.promise;
}

@lukehoban lukehoban closed this Apr 9, 2015

@getify

This comment has been minimized.

getify commented Apr 9, 2015

I find these two options similar in overall complexity

Just a quick clarification: they are similar in a single line function case. In a larger function, where you're indenting dozens of lines of code inside the promise constructor, it's more painful. The pain adds up if there's nested promise creation, etc. Also, the pain is increased if you're dealing with this bindings, where you're forced into arrow functions or var self = this junk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment