Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Allow passing objects to util.promisify #174

Closed
benjamingr opened this issue Sep 1, 2017 · 6 comments
Closed

Allow passing objects to util.promisify #174

benjamingr opened this issue Sep 1, 2017 · 6 comments

Comments

@benjamingr
Copy link
Member

This is a proposal to extend util.promisify to accept objects in addition to promises.

Currently, a user has to type util.promisify for every function in a module they're using. This leads users to prefer userland solutions like bluebird which provide a more ergonomic method that converts entire APIs at once.

This behavior was initially planned for nodejs/node#12442 but we decided to defer it until the initial function behavior was used for a while and we saw the response.

So far the response I've received from communities I polled was generally very positive for promisify, and people often asked about the object variant.

I think now would be a good time to add it:

const fs = util.awaitable(require(fs));
async function foo() {
  const result = await fs.readFile("hello.txt"); // file contains "World"
  console.log("Hello", result);
}
foo(); // logs `Hello World`

Pinging @addaleax who wrote the code for util.promisify and did a considerable amount of design for it.

I'm posting it here for discussion about it and as a place for bikeshedding syntax or making objections for possible designs.

@jasnell
Copy link
Member

jasnell commented Sep 1, 2017

While I definitely get the utility of this, there's a danger here in that the API could end up doing entirely the wrong thing for functions that do not follow the error back pattern and do not provide their own custom conversion logic.

For instance, what should the result of the following be:

class Foo {
  a(err, arg) {}
  b(num, options) {}
}

const foo = util.awaitable(new Foo());

foo.b(1, {}); // ?? Is this awaitable?

If we can clearly define the semantics and what the users would expect, then I'm all for this.

@benjamingr
Copy link
Member Author

@jasnell it would be identical to what util.promisify does for each property of the object. Something like:

for(const prop of Object.getOwnPropertyNames(Foo.prototype)) {
  if(prop === 'constructor') continue; // .prototype.constructor
  const old = Foo.prototype[prop];
  Foo.prototype[`${prop}Async`] = util.promisify(old); // should probably be defineProperty
}

Or alternatively without modifying the original object.

@mcollina
Copy link
Member

mcollina commented Sep 1, 2017

I'm generically 👎 on overly overloaded functions. This seems too much, can we do it in a different function?

Moreover, I do not see why it cannot exist in the ecosystem now that the basics of util.promisify  are in core (and implemented in C++).

@addaleax
Copy link
Member

addaleax commented Sep 2, 2017

I'm generically 👎 on overly overloaded functions. This seems too much, can we do it in a different function?

Seconded

Moreover, I do not see why it cannot exist in the ecosystem now that the basics of util.promisify are in core (and implemented in C++).

I see a single reason, but maybe it’s a biggie (maybe not): It solves the problem @jasnell is pointing out for core modules in a way that can’t be done in userland without continuously keeping up to date with core, because it would allow us to differentiate between the methods on core modules/objects that could reasonably be promisified vs those who can’t. (Simple example: WritableStream#write() can be promisifed, WritableStream#setDefaultEncoding() cannot).

That might still be a lot of work, but it should be pretty doable.

@mcollina
Copy link
Member

mcollina commented Sep 2, 2017

I see a single reason, but maybe it’s a biggie (maybe not): It solves the problem @jasnell is pointing out for core modules in a way that can’t be done in userland without continuously keeping up to date with core, because it would allow us to differentiate between the methods on core modules/objects that could reasonably be promisified vs those who can’t. (Simple example: WritableStream#write() can be promisifed, WritableStream#setDefaultEncoding() cannot).

I agree that it is a lot of work. However, I think we should provide some strategy to provide awaitable functions for the main modules of core. We might make that mechanism public, but it might be something that should be very specific to core.

I would reverse the process:

  1. design a new internal utility to get a promisified version of a core module
  2. release that promisified layer as experimental
  3. make the utility public once its proven to work for us when we lift the experimental flag

@Trott
Copy link
Member

Trott commented Sep 8, 2017

I'm going to close this, but feel free to open another issue in a relevant active repository (TSC perhaps?) and include a link back to this issue if this is a subject that should receive continued attention. Recent governance changes mean this repo probably shouldn't be used anymore. Sorry for the inconvenience.

@Trott Trott closed this as completed Sep 8, 2017
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants