-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add `util.promisify(function)` for creating promisified functions. Includes documentation and tests. Fixes: nodejs/CTC#12 PR-URL: #12442 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: William Kapke <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Teddy Katz <[email protected]>
- Loading branch information
Showing
5 changed files
with
222 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
'use strict'; | ||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const fs = require('fs'); | ||
const vm = require('vm'); | ||
const { promisify } = require('util'); | ||
|
||
common.crashOnUnhandledRejection(); | ||
|
||
const stat = promisify(fs.stat); | ||
|
||
{ | ||
const promise = stat(__filename); | ||
assert(promise instanceof Promise); | ||
promise.then(common.mustCall((value) => { | ||
assert.deepStrictEqual(value, fs.statSync(__filename)); | ||
})); | ||
} | ||
|
||
{ | ||
const promise = stat('/dontexist'); | ||
promise.catch(common.mustCall((error) => { | ||
assert(error.message.includes('ENOENT: no such file or directory, stat')); | ||
})); | ||
} | ||
|
||
{ | ||
function fn() {} | ||
function promisifedFn() {} | ||
fn[promisify.custom] = promisifedFn; | ||
assert.strictEqual(promisify(fn), promisifedFn); | ||
assert.strictEqual(promisify(promisify(fn)), promisifedFn); | ||
} | ||
|
||
{ | ||
function fn() {} | ||
fn[promisify.custom] = 42; | ||
assert.throws( | ||
() => promisify(fn), | ||
(err) => err instanceof TypeError && | ||
err.message === 'The [util.promisify.custom] property must ' + | ||
'be a function'); | ||
} | ||
|
||
{ | ||
const fn = vm.runInNewContext('(function() {})'); | ||
assert.notStrictEqual(Object.getPrototypeOf(promisify(fn)), | ||
Function.prototype); | ||
} | ||
|
||
{ | ||
function fn(callback) { | ||
callback(null, 'foo', 'bar'); | ||
} | ||
promisify(fn)().then(common.mustCall((value) => { | ||
assert.deepStrictEqual(value, 'foo'); | ||
})); | ||
} | ||
|
||
{ | ||
function fn(callback) { | ||
callback(null); | ||
} | ||
promisify(fn)().then(common.mustCall((value) => { | ||
assert.strictEqual(value, undefined); | ||
})); | ||
} | ||
|
||
{ | ||
function fn(callback) { | ||
callback(); | ||
} | ||
promisify(fn)().then(common.mustCall((value) => { | ||
assert.strictEqual(value, undefined); | ||
})); | ||
} |
99da8e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌 * 9001 — so excited for this.
Will it be in 8.x, then?
99da8e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Aendrew Yes! :) Have an RC: https://nodejs.org/download/rc/v8.0.0-rc.0/ The 8.0.0 release itself is planned for/by May 30th. :)
99da8e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Aendrew Yes it will, very exciting! I wrote some of my first thoughts on Medium.
99da8e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this in the
util.
name space and not in thePromise
namespace, which seems far more natural?99da8e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it’s an utility function. If you have a better place in Node core where it should be (
Promise
doesn’t count for that, see below), we can revisit that. I haven’t seen any, though. :)Because
Promise
is a language built-in, not something that Node offers. We try to avoid messing with built-in object as much as possible, so that users know they may rely on those having the same features available in other environments as Node.99da8e8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@addaleax Thanks for the prompt and clear response.
I appreciate the attempt to be compatible with other JavaScript environments. I'm part of the group of largely backend-only developers, so I don't notice the compatibility differences with JavaScript engines in browsers, but I do use a mix of native and
bluebird
promises on the backend. When it comes to comparing tobluebird
and other promise libraries, the choice of theutil
namespace is confusingly different when there's already a precedent forPromise.promisify
existing and working exactly the same asutil.promisify()
.What would be more useful to be would be:
This solution be non-conflicting-- an understandable goal-- but still clear and convenient. Loading most Promise methods from
Promise
but then loading just one fromutil.
would be both confusing and inconsistent.In my example, I use the new
node.
namespace for a sub-namespace that might conflict with a native namespace, but that has been modified or extended for Node.js.