Method with no parameters will error

[Siyfion]

So as per the Meteor guide, if you use mdg:validated-method to create a method that takes no parameters, you still must provide a validator, like so:

validate: new SimpleSchema({}).validator()

But there is an issue then if you then call that method with no parameters at all. e.g. calling

Collection.methods.methodName.call()

will error. As the validator, expects an object as the first parameter. Obviously just changing this to:

Collection.methods.methodName.call({})

fixes the issue, but it's a little awkward.

[stubailo]

@Siyfion I think the most reasonable thing to do in this case is set validate: null. Unfortunately, that doesn't enforce no arguments - it just doesn't validate at all.

What would be the best way to fix this, do you think?

[Siyfion]

Ideally, enforcing no arguments would be the prefered approach, but failing that, just adding a short bit to the readme / Meteor Guide that mentions the validate: null option would suffice for now. Not sure if you've tried it, but the error given isn't particularly clear, had me scratching my head for a few minutes!

[stubailo]

@Siyfion hey, sorry for digging up this issue, but what do you think of a solution where if you don't pass any arguments to call, it defaults to a single {} argument?

[Siyfion]

@stubailo That sounds perfectly reasonable!

[rhettlivingston]

I just encountered this in writing my own test cases and was considering whether to post an issue here or on SimpleSchema when I read this issue.

In my case, the first failure was with a test that requires no arguments like the above example. In another case that fails, all of my schema elements were defaulted and validator had been called with "{ clean: true, filter: false }" so that the defaults would be set.

In the first case, I can understand viewing the problem as not a problem in SimpleSchema. After all, SimpleSchema was designed for validating records to be put in a database. It would be a bit weird for someone to have a database schema required to be empty. We are stretching SimpleSchema's use cases in using it to validate parameters in a method call.

However, in the second case, I'm not sure we are out of bounds in expecting SimpleSchema to accept an undefined document and validate it (actually defining it in the process with defaultValues). I say this because the error I get when calling it with ".validator({ clean: true, filter: false })" comes from the "clean"ing stage of the code that would implement my defaultValues for me. There is an "in" operator in that code that tries to work with the "undefined" doc and throws an exception. If I were calling SimpleSchema's clean explicitly in preparation for inserting the object into a collection, I'd get the same failure.

I took the trouble to comment out the "in" operator in SimpleSchema (it was in a check designed to warn if someone is still using $pushall) and a parameter check in Validate that was explicitly requiring that the doc be defined and my test case worked with SimpleSchema behaving as I desired. Perhaps other things would be broke.

I'm all for the patch you've shown above being implemented somewhere, either in ValidatedMethod or SimpleSchema!!!!!!!!!!!

People who know this better than I do need to decide whether they may be covering up another package's issue by patching it here or whether the changing of the argument from undefined to {} could be unexpected behavior to someone writing a "run" block.

[stubailo]

I'm going to implement the default {} arg and document it.

[kachkaev]

Hey guys! Any updates on this issue? This is likely a very easy pick, so would you mind updating the repo? A proposal by @stubailo is what I'm looking for – it is a bit odd to use Meteor.call('x', {}) when you just could Meteor.call('x').

[deanrad]

We meet again @stubailo ! This line is giving me grief: https://github.com/meteor/validated-method/blob/master/validated-method.js#L67 - The package is hard-coded to pass only one argument (an object) through! Why oh why?? It's the cause of the above problem, as well as mine, where method.call(a, b) is not supported. I'm already working on a 'companion package' (is that what you want, vs a PR?), but I'm surprised to find this level of hard-coding that seems like it should be generalized in this package.

[stubailo]

This package has an intentional opinion that methods should only take keyword arguments. If you want to pass multiple arguments, they should be passed as properties of the single object argument.

[deanrad]

Yes @stubailo come to think of it, that's a good opinion to enforce with ES6 keyword args.

[BudgieInWA]

Could ValidatedMethod provide a "no arguments validator" that enforces no arguments and throws the appropriate ValidationError. Something like this:

ValidatedMethod.noArgsValidator = () => {
  if (arguments.length !== 0) {
    throw new ValidationError(whateverTheErrorIsSupposedToLookLike);
  }
}

// Now we can:
const myMethod = new ValidatedMethod({
  name: 'myMethod',
  validate: ValidatedMethod.noArgsValidator,
  run() {
    // ...
  }
}

You would probably also want validation to pass if the only argument was an empty object.