aproba-overload
rhettl opened this issue · 9 comments
Hey,
I found your package tracing makara and adaro a couple months back and I found it really useful in a project I am now working on, so THANK YOU 👍 !
That said, I found my project to benefit from having a T/F returned for use in cases of function overloading via optional params.
TL;DR: I wrote a wrapper function which adds OR(|) and returns T/F if meets or doesn't meet schema. Do you want it? I'll make a PR.
My use case if a router function takes in multiple configurations.
function (options, [callback]) {}function (template, dataObj, [options], [callback]) {}
After taking the information in, I parse into a single options object and call an internal function to do the actual work.
var q = require('q');
var extend = require('deep-extend');
var probe = require('./aproba-overload');
module.exports = {
_compileOptions: {
engine: null,
template: null,
data: null
},
_compile: function (options, callback) {
var d = q.defer();
// Do compile and resolve/reject promise
return d.promise.nodeify(callback);
},
/**
* Compile template
*
* @param {String|Object} template Either template name or options object
* @param {Object} [data] data for template rendering
* @param {Object} [options] optional object for options
* @param {Function} [callback] optional callback, returns promise of null
*
* @return {q.Promise|null} promise if no callback, null if returned.
*/
compile: function (template, data, options, callback) {
var args = arguments;
var opts = extend({}, module.exports._compileOptions);
if (probe('O|OF', args)) { // Options with optional callback
extend(opts, args[0]);
callback = args[1];
} else if (probe('SO|SOF', args)) { // template name and data, optional callback
opts.template = args[0];
opts.data = args[1];
callback = args[2];
} else if (probe('SOO|SOOF', args)) { // template name and data, additional template options, optional callback
extend(opts, args[2]);
opts.template = args[0];
opts.data = args[1];
callback = args[3];
} else {
throw new Error('Invalid Arguments'); // need more descriptive error message.
}
return module.exports._compile(opts, callback);
}
};Here is my wrapper:
/**
* Filename: aproba-overload.js
* Created by rhett on 2/22/16.
*/
'use strict';
var aproba = require('aproba');
var validate = module.exports = function (schema, args, throwErr) {
// Enable OR syntax
if (/.+\|.+/.test(schema)) { // example schema = 'O|OF|O*F'
var t = schema.split(/\|(.+)/); // t = ['O', 'OF|O*F', '']
// return recursive for schema = 'O' OR recursive for schema = 'OF|O*F' <= which has another recurse
return validate(t[0], args, throwErr) || validate(t[1], args, throwErr);
// Todo: problem with throwErr here; If true passed it may not reach second call. Rethink this combo
}
try {
aproba(schema, args);
} catch (err) {
if (!throwErr && // pass-through requested
[
'EMISSINGARG',
'EUNKNOWNTYPE',
'EINVALIDTYPE',
'ETOOMANYARGS'
].indexOf(err.code) // Catch only known errors
) {
return false; // Doesn't meet schema
} else {
throw err; // pass-through or other, unexpected error
}
}
return true; // no problems, success
};At the moment it is just a file in my libs, but I plan to later make into a tested module. Alternatively, I can make a pull request to add this into aproba if you are interested in having it.
If you think this code will be beneficial to you code base, tell me how you think it would be best to add. Otherwise, I'll just make a new package.
Have a great day,
I'm of mixed feelings… there is absolutely utility to what you describe, but OTOH part of the driving purpose of this module is to keep it as simple as possible.
So I think I'd prefer to see this as a a separate module.
If you want to release your thing as a separate module and PR a doc update here with a pointer over to your new module for folks who need those features, I'd take that. =)
I'm still ambivalent which is why I've left this open. I'm deeply tempted to add | syntax to core…
Well if it helps, on a second look as to how to add to core, I can't find a simple way.
return validate(t[0], args, throwErr) || validate(t[1], args, throwErr);There is a flaw. in my usage, since throwErr is always false in my code I am unaffected. The problem comes in when you trace my code with throwErr = true. For example
let args = ['hello', () => {}];
probe('S|SO|SF|SOF', args, true);Should trigger "SF", but in the first iteration of the recursive return line (quoted above) it will throw ETOOMANYARGS error and not continue to iteration 2 or 3. I don't see a simple way to do this without the try/catch block or a total rewrite away from recursion.
Additionally, I can't think of a way to handle errors as specifically as you have there. In the example given, there would be 1 success and 3 errors: ETOOMANYARGS, EINVALIDTYPE, and EMISSINGARG. Let's say it caches the results until it finds a successful one, then parses the errors if it fails all. Which of the errors should it throw. Considering, that your errors are presently called clearly and very clearly denote the problem, I think it would be a detriment to change that.
Can you think of a solution to these problems? Having pipe-notation (ex: S|SO) is really useful for situations where you want to use the kriskowal/q library's promise.nodeify(done) (linked here) or any place you want an optional options object or just overloading in general, but I agree that two of this module's best features are its simplicity of purpose and code and its clearness of error messages.
Ok, I've given this some thought. I think the way I would approach this in core would be to make a list of the valid signatures and then progressively filter it down. If we ever end up with no signatures left then that's an error.
So for your example we would:
signatures = ['S', 'SO', 'SF', 'SOF']- Filter by number of arguments (2):
['SO', 'SF'] - Filter by first argument type, both match:
['SO', 'SF'] - Filter by second argument type:
['SF'] - No more args and we still have some signatures: SUCCESS!
Say the second argument had been another string…
Then we would have failed at step 4 and would have reported that it could be an object or a function, but we were given a string.
Another wrinkle is that the error type already implies a variable length signature. But I think that can be folded in without too much trouble.
We do lose the missing-arg error this way, but extending the "wrong number of args" error to include what the valid signatures were seems plausible.
@rhettl If you want to take a look at https://github.com/iarna/aproba/tree/alternates it has a first pass implementation of this. No docs and the tests haven't been updated to include the new feature, but it works as described here.
Perhaps use lowercase letters to indicate optional fnc params.
function (options, [callback]) {}==>aproba('Of')function (template, dataObj, [options], [callback]) {}==>aproba('SOof')
I am only considering using this module so please do not consider my comment as an endorsement of this feature request.
@dhollenbeck That seems... plausible, although I fear the slippery slope of creating too complicated a grammar for the expressions. I think I'd prefer to start by updating docs & tests for the alternatives branch and landing that and seeing how it feels.
If it needs a shortcut then we can do something like what you suggest and have shortcut expressions that get expanded into alternatives like I do for E in the alternatives branch.
That is EO internally becomes EO|E|ZO
And your example SOof would become SOOF|SOZF|SOOZ|SOO|SOZZ|SO I guess?
But I think what we really want there would be SO|SOO|SOF|SOOF?
Double optionals are always hard to express and I'm not super comfortable with how they increase complexity. I kind of like the alternatives form because they make that complexity super apparent.
What I've described is merged and published. As this implements the original request, I'm going to close this. If after trying this out any of you all are still interested in seeing sugar for optionals, please feel free to open a new issue.