Adding support for promises
Closed this issue · 13 comments
I'd like to request support for the Promise API. Your code example together with async / await would look like following then.
const securePassword = require('secure-password')
// Initialise our password policy
const pwd = securePassword()
const doHashingAndVerify = async function () {
const userPassword = Buffer.from('my secret password')
// Register user
const hash = await pwd.hash(userPassword)
// Save hash somewhere
const result = await pwd.verify(userPassword, hash)
if (result === securePassword.INVALID) return console.log('Imma call the cops')
if (result === securePassword.VALID) return console.log('Yay you made it')
if (result === securePassword.VALID_NEEDS_REHASH) {
console.log('Yay you made it, wait for us to improve your safety')
const improvedHash = await pwd.hash(userPassword)
}
}
doHashingAndVerify()
.then(() => console.log('succesful'))
.catch(err => throw err)
Really enjoyed your JSConf talk btw!
I'm a bit worried about the burden of testing and maintaining this, as I haven't used promises or async/await much myself
Okay, I'll probably create a simple wrapper pkg over the next few days then.
I have a new feature coming up for queueing and cancelling, so maybe hold off until that lands :)
Instead of a wrapper which needs to be maintained, wouldn't it be easier to simply promisify the module in your code via util.promisify
?
Sure, but let's keep it backwards compatible with LTS
@matteodem I think @watson was implying that people can promisify the functions themselves as to relieve the maintenance burden on me, and avoid issues with different Node versions. I'd like if for Node not to become a peerDependency more than it already is :)
ah.. yeah that makes sense, then let's leave it at that
For anyone coming back in the future; this is the function that was mentioned: https://nodejs.org/api/util.html#util_util_promisify_original
I modified the original code in this issue to use promisify
(Node v8.6.0).
const securePassword = require('secure-password');
const { promisify } = require('util');
const pwd = securePassword();
const hashAsync = promisify(pwd.hash);
const verifyAsync = promisify(pwd.verify);
const doHashingAndVerify = async function () {
const userPassword = Buffer.from('my secret password');
const hash = await hashAsync(userPassword);
const result = await verifyAsync(userPassword, hash);
if (result === securePassword.INVALID) return console.log('Imma call the cops');
if (result === securePassword.VALID) return console.log('Yay you made it');
if (result === securePassword.VALID_NEEDS_REHASH) {
console.log('Yay you made it, wait for us to improve your safety');
await hashAsync(userPassword);
}
}
doHashingAndVerify()
.then(() => console.log('succesful'))
.catch(err => console.log(err))
and I get the following error: "Error: opslimit must be a number".
Anybody with a similar problem and a solution to that issue?
@mrksbnch It seems that util.promisify
changes the context of the wrapped function, so that this
no longer references the securePassword
instance (the this
part here), and instead refers to the global scope. I don't know much about promises or how promisify works, but it seems that the issue lies here, where original
is called with a this
, which in that context refers to the global scope:
@emilbayes Thanks for looking into this! Then I guess we have to use...
const pwd = securePassword();
const hashAsync = promisify(pwd.hash.bind(pwd));
const verifyAsync = promisify(pwd.verify.bind(pwd));
... which is working just fine.
Promises are now available in 3.1.0