Open simonepri opened 6 years ago
Good work! I would actually go ahead and remove install
and uninstall
.
I'm happy help :) Will open a PR in the next few days. I have a few questions first though.
Is this finialised?
const upash = new UPASH({
argon2: require('@phc/argon2'),
pbkdf2: require('@phc/pbkdf2')
}, {default: 'argon2'});
Is there ever going to be any other options passed in the second object? If not I would favour this approach:
const upash = new UPASH({
argon2: require('@phc/argon2'),
pbkdf2: require('@phc/pbkdf2')
}, 'argon2');
Also what do we want the behaviour to be if no default is specified?
"I'm happy help :)"
Thank you @gavinhenderson !!!
"Also what do we want the behaviour to be if no default is specified?"
It should definitely throw an error.
"Is this finalised?"
Not sure, I'm open to suggestions.
I was also thinking at something like that:
const upash = new UPASH({
argon2: {algoritmh: require('@phc/argon2'), default: true}
pbkdf2: {algoritmh: require('@phc/pbkdf2'), options: {someParamToOverride: 'somevalue'}}
});
But it seems over complicated. I want to make the API as easy as possible
But it seems over complicated. I want to make the API as easy as possible
I completely agree, people (including me) are turned off to a package at the first sign of a weird API so we want to avoid that as much as possible.
Not sure I like the idea of specifying the default in the algorithm object as this could to lead to people entering multiple defaults which we could handle but I think its unnecessary. I would favour the one you originally suggested:
const upash = new UPASH({
argon2: require('@phc/argon2'),
pbkdf2: require('@phc/pbkdf2')
}, {default: 'argon2'});
This allowing for easy expansion of the options object which I like. Maybe would could make it more clear in the usage guide (when we get to that) and do something like:
const options = { default: 'argon2' }
const upash = new UPASH({
argon2: require('@phc/argon2'),
pbkdf2: require('@phc/pbkdf2')
}, options);
@gavinhenderson Yeah I totally agree, I was just throwing ideas.
Description
Expose a constructor to allow the users to have multiple instance of upash instead of having it as a singleton.
Examples
Notes
Probably it makes sense to remove
install
anduninstall
methods and add agetDefault
method. This would lead to a breaking change.cc @mcollina