feat: function argument validation

[bcoe]

Too help folks consuming the yargs API to better understand when they've provided illegal arguments to functions, this pull request introduces an approach to function argument validation.

I think this is a pretty clean API, and would potentially like to release it post-hoc as a library.

[iarna]

Why the [].slice.call(arguments)? This seems horrifically ugly. =D (Uhm, in the nicest way possible.)

Is argsert trying to forEach the args part? Why not change that to a C-style for loop and save yourself this weirdness?

[iarna]

Ah, no, I see that you're doing the same thing in argsert itself, so this is thankfully unnecessary.

[bcoe]

definitely a bit ugly, but in our discussion in coding it sounded like there's a pretty big performance hit with passing in arguments -- I opted for ugly and fast.

[iarna]

That doesn't improve the performance. Passing it to slice still requires the object be created.

[bcoe]

@JaKXz @nexdrew, etc., I believe I've added argument validation for the whole public API surface; making this pull request ready for prime-time ...

regarding the ugly [].slice.call(arguments) syntax being discussed by @iarna and I, this comes from a conversation with @isaacs and @chrisdickinson ... tldr; passing arguments into another function is a performance hit for the compiler.

[iarna]

Using arguments that way is exactly the same performance hit for the compiler.

[iarna]

I recommend you read Bluebird's Optimization Killers docs, it goes into great detail.

(The only bit that's a little outdated is reassigning, which is only an issue if you don't 'use strict')

[bcoe]

@iarna hmmm, I'd rather avoid adding a build step/macro ... is your thinking "might as well just pass arguments, since slice is slow too"?

[isaacs]

Look, your api is not requiring that you pass the actual arguments object. As long as you're ok with taking an array as well, it's fine. Just have a caveat in your docs. Most programs will call this once on startup, not in a fast loop. It's probably fine.

If you want to avoid passing arguments at all, you could still test for unwanted args by passing arguments.length.

function argcert (spec, len) {
  var args = new Array(Math.max(0, arguments.length - 2))
  for (var i = 2; i < arguments.length; i++) {
    args[i - 2] = arguments[i]
  }
  if (len < args.length) {
    // missing arguments!
  } else if (len > args.length) {
    // extra arguments!
  }
}

function myFunction (a, b, c, d) {
  argcert(stringDefinition, arguments.length, a, b, c, d)
}

You could do the same thing without the loop using a splat:

function argcert (spec, len, ...args)

But really, that's a probably just less convenient API, and for a dubious performance benefit, so why bother?

I'm really academically curious about the optimization differences between [].slice.call(arguments), Array.prototype.concat.apply([], arguments) and Array.apply(Object.create(Array.prototype), arguments), and the newer Array.from(arguments).

[iarna]

Yeah, basically what @isaacs said. I would write the code such that the args argument can be an array or a arguments object. That is, just do c-style iteration over it. And then document that folks call it like:

function (a, b, c) {
  argcert(spec, arguments) // when perf doesn't matter
  argcert(spec, [a, b, c]) // when it does
}

And beyond that, unless you're calling a function (tens/hundreds of) thousands of times it really doesn't matter if it's deopted. Unoptimized v8 is still fast enough, most of the time.

[bcoe]

@iarna @isaacs, having discussed this topic with @JaKXz too; and having read the bluebird docs @iarna shared, I think @isaacs' example of:

argcert(stringDefinition, arguments.length, a, b, c, d)

Is what I'd like to move to; gives me everything I want, isn't too much more messy, and doesn't do something slow just to have a slightly more magic API signature.

This 🚲 🏠 taught me something about Node.js performance, which rocks.

[bcoe]

declaring 🚲 🏠 painted.