Skip to content
This repository was archived by the owner on Mar 31, 2018. It is now read-only.
This repository was archived by the owner on Mar 31, 2018. It is now read-only.

util.promised - Callbacks and async/await  #5

Closed

Description

@benjamingr

With async/await landing in v8 callback users should not have their workflows harmed.

I'm wondering if core should offer a util.promised method that wraps a callback returning method in such a way it can be used with async/await - effectively allowing non-promise users interact with async/await. It would work similarly to Promise.fromCallback:

async function doBar() {
    var result = await promised(fs.readFile)("hello.txt");
}

I'm not sure about the name since promised might come off as polarizing to callback users who would not want to be aware of the internals of async/await using promises.

Also, it's worth mentioning it doesn't have to return an actual promises but rather just a thenable since those are consumed by async/await.

Activity

  1. changed the title [-]`util.promise` - Callbacks and async/await [/-] [+]`util.promised` - Callbacks and async/await [/+] on Feb 15, 2016
  2. chrisdickinson commented on Feb 15, 2016

    @chrisdickinson
    Contributor

    We may also consider that this wrapper should cast returned promises from the wrapped API into native promises — to make it easier to consume promises from, e.g., knex, if one doesn't want knex's bluebird Promises to propagate through their code.

  3. chrisdickinson commented on Feb 15, 2016

    @chrisdickinson
    Contributor

    Marking this as "not-blocking" for now. If anyone objects, please let me know and I'll be happy to change it!

  4. petkaantonov commented on Feb 15, 2016

    @petkaantonov
    Contributor

    What about asAwaitable? And talk about awaitables instead of promises.

  5. phpnode commented on Feb 15, 2016

    @phpnode

    What about just awaitable. I'm not sure it should assimilate thenables - a thenable can already be awaited, so they should just pass through. Assimilation is taken care of by Promise.resolve().

    const get = awaitable(function(url, cb) {
      request(url, cb);
    });
    
    const get2 = awaitable(get);
    
    get === get2; // true because get is already thenable
    
    async function go () {
     const data = await get("http://example.com/");
     return data;
    }
  6. benjamingr commented on Feb 15, 2016

    @benjamingr
    MemberAuthor

    Yes, awaitable and awaiters instead of promises sounds like very good terminology to me. We don't need a full promise implementation or actual promises at all for this :)

  7. MadaraUchiha commented on Feb 15, 2016

    @MadaraUchiha

    So... you're basically (almost) saying that you want bluebird's Promise.promisify(fn) in core.

  8. nolanlawson commented on Feb 16, 2016

    @nolanlawson

    I think it's a bit silly to avoid the word "promise" since that's fundamentally what async/await is about – e.g. an async function is just a function that returns a promise. Also, the word "promise" will be inevitable when writing async/await code:

    // `Promise.all` has replaced the now-defunct `await *`
    var results = await Promise.all(arrayOfPromises);
  9. benjamingr commented on Feb 16, 2016

    @benjamingr
    MemberAuthor

    So... you're basically (almost) saying that you want bluebird's Promise.promisify(fn) in core.

    Not at all. promisifys goal is to convert a callback API to a promises one. This is to get async/await working with callback APIs.

    I think it's a bit silly to avoid the word "promise" since that's fundamentally what async/await is about

    Technically we could have an Awaitable.all but I do agree that's kind of silly. We really want to accommodate callback users so they don't get "left behind" in the past. I think we need more feedback from these users.

  10. MadaraUchiha commented on Feb 16, 2016

    @MadaraUchiha

    Yes, we do.

    The way I see it, the vast majority of people who aren't using Promises would not use async functions either because they either

    1. Don't know enough to progress beyond the basics of what the tutorials taught them
    2. Are forced to support browsers without Promise support (and thus, without async/await support)

    There probably are cases where people just choose (for some reason) to not use Promises in their code, be it by technical debt or a general dislike for Promises. If that's the only audience we're aiming at, I don't think it's worth the effort of putting it into core. Just use a library or implement it yourself.

  11. benjamingr commented on Feb 16, 2016

    @benjamingr
    MemberAuthor

    @MadaraUchiha

    If that's the only audience we're aiming at, I don't think it's worth the effort of putting it into core. Just use a library or implement it yourself.

    There are plenty of people who choose not to use promises. These people should be able to enjoy async/await and making sure they have a good programming experience when async/await lands is a priority.

  12. chrisdickinson commented on Feb 16, 2016

    @chrisdickinson
    Contributor

    @benjamingr:

    Not at all. promisifys goal is to convert a callback API to a promises one. This is to get async/await working with callback APIs.

    Given that we'd have to introduce a third API (in addition to the currently-internal but externalizable promisify and callbackify) to turn callback-based functions into .then-ables, I believe it would be best to focus on how best to expose the internal promisify function to users. One of the stated goals of exposing a Promise-returning API from core is to address the needs of upcoming async/await users, and I don't see us preserving any value for ourselves (or our users) by returning non-Promise objects.

  13. benjamingr commented on Feb 16, 2016

    @benjamingr
    MemberAuthor

    @chrisdickinson I tend to agree with that too but I mostly use promises so I don't have that problem.

    I'd like to know if callback proponents like @Raynos or @rvagg are interested in such an API and if they intend to use async/await at all.

  14. nolanlawson commented on Feb 16, 2016

    @nolanlawson

    There are plenty of people who choose not to use promises. These people should be able to enjoy async/await

    Choosing to use async/await but not Promises is like choosing to be a vegetarian but eating a big corn dog. 😉 Just because you can't "see" the meat (Promises) doesn't mean it's not there...

  15. omsmith commented on Feb 16, 2016

    @omsmith

    Benefits of making util.promised public:

    • Can be used for 3rd party libraries
    • Can ignore the core-provided promise modules
      • Or could "replace" core-provided promise modules, technically, I guess

    Cons of making util.promised public:

    • "Encourages" (at least in the example this issue proposed ) inefficient use of the function
      • (i.e. proper usage would be const awaitableReadFile = promised(fs.readFile); async function() { await awaitableReadFile(...); } in order to avoid recreating the shim every time)
        • Memoizing promised would make it less horrible
  16. Raynos commented on Feb 16, 2016

    @Raynos

    @benjamingr as a callback user I want whatever is the most performant.

    I suspect async/await will be too slow and I care more about performance then I care about synchronous looking code.

  17. benjamingr commented on Feb 16, 2016

    @benjamingr
    MemberAuthor

    @Raynos I presume that if it's implemented the same way as generators it would be fast. Slower than raw callbacks optimized, faster than things that use modules like async.

    I doubt you'd have actual performance gains from it since you already probably avoid closures, pass only needed parameter and such - but I suspect most users who use closures in their code would see a small performance boost from async/await.

    In either case, I doubt there would be a noticeable performance discrepancy between code written with await and code written without.

    Are you saying that as a callback user this completely disinterests you? If it's within 20% speed of a "raw callback" solution would that change it?

    We have everyone involved here, from the person implementing async/await in v8 to the people working on this PR to the people working in post-mortem and the error people.

    I'm asking what you'd like to see "in your wildest dreams". Of course "I don't care about any of this or synchronous looking code" is also a completely valid response.

  18. Raynos commented on Feb 16, 2016

    @Raynos

    @benjamingr

    Please benchmark before making statements. I wouldn't be suprised if generators are an ORDER OF MAGNITUDE slower then callbacks.

    If I can get a 2x perf boost from not using async/await then i'll use callbacks.

    If async/await is fast then I'll use it. I think it's very nice, especially for authoring integration tests or implementing endpoints.

  19. phpnode commented on Feb 17, 2016

    @phpnode

    @Raynos this goes both ways. There are lots of benchmarks out there that show that callbacks and generators are much closer than that, e.g. the bluebird benchmarks. If you have evidence to the contrary please provide it.

  20. benjamingr commented on Feb 17, 2016

    @benjamingr
    MemberAuthor

    @Raynos

    Please benchmark before making statements. I wouldn't be suprised if generators are an ORDER OF MAGNITUDE slower then callbacks.

    I wouldn't have been surprised either but if you consider their implementation as state machines they actually make for some pretty efficient code. Mainly because no closures are needed to write code that would have been convenient to write closures with. So I assume you would not gain a performance boost since you probably avoid nested callbacks anyway and name your callbacks - but everyone who nests callbacks will get a performance boost.

    For example, I'd bet that:

    for(var i = 0; i < arr.length; i++) {
        yield fn(arr, _); // _ substitutes CB and is a parameter is a generator
    }

    Is faster than a recursive alternative like:

    function sequence(fn, arr, cb) {
        function run(results) {
           if(results.length === arr.length) cb(null, results);
           fn(arr[results.length], (err, data) => { 
                if(err) return cb(err);
                results.push(data);
                run(results);
           });
        }
        run([]);
    }

    You would not write code like sequence above, so again - I doubt you'd get a performance boost but I wouldn't rule them out altogether - especially in paths in the code that are not hot.

    Now - if you were to use async/await what syntax would you like?

  21. petkaantonov commented on Feb 17, 2016

    @petkaantonov
    Contributor

    Naming callbacks doesnt get rid of any allocation, apis dont provide a way to pass a context object so closures are unavoidable

  22. benjamingr commented on Feb 17, 2016

    @benjamingr
    MemberAuthor

    @petkaantonov I mean something like this:

    function a(cb) {
        getV((err, data) => err? cb(err) : b(data, cb));
    }
    function b(v, cb) {
       // the closure over 'v' inside getV does not need to exist here, unlike if we nest
    }

    They're not completely avoidable, but it minimizes it significantly for the common user.

  23. ChALkeR commented on Sep 9, 2016

    @ChALkeR
    Member

    Related: nodejs/CTC#12

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions