Add 'some' and 'every' as additional modifiers

[adityasharat]

Proposing to add some and every as new modifiers for testing array elements.

v8n()
  .some.number()
  .even()
  .test([1, 2, 3]); // true

v8n()
  .some.number()
  .even()
  .test([1, 3]); // false

v8n()
  .every.number()
  .even()
  .test([1, 2, 3]); // false

v8n()
  .every.number()
  .even()
  .test([2, 4]); // true

An argument can be made for the following instead:

[1, 2, 3].some(i => v8n().number().even(i));

But personally, I find the former more elegant.

p.s. I can work on this feature request.

[sbarfurth]

Uhm, I don't think the position of some is sound in your example. I believe the modifier should be on the thing that is checked. As you wrote it one could assume that some applies to number, which would mean both [1, 2, 3] and [1, 3] would be correct.

Generally I think the idea is nice. If you want to submit a PR for discussion do feel free. I'm not a member, but the feature is useful enough to be a core feature in my opinion.

This would be better syntax:

v8n()
  .number()
  .some.even()
  .test([1, 2, 3]); // true

v8n()
  .number()
  .some.even()
  .test([1, 3]); // false

v8n()
  .number()
  .every.even()
  .test([1, 2, 3]); // false

v8n()
  .number()
  .every.even()
  .test([2, 4]); // true

Modifiers so far only apply to the rule they are attached to, so they would need to be repeated for number() and even() to both be modified.

Please do implement it though and add some tests to show useful examples and syntax.

[adityasharat]

Ah, yes, the not modifier

is used before a rule function call and will invert that rule meaning, making it expect the opposite result

• Is it a defined contract for all modifiers? (not is the only modifier right now)
• Should there be global modifiers? (in my opinion some and every begs for global modifiers)

I will start working on an implementation to share so it is easier to discuss. Also, thank you for the opportunity to try and contribute. 😃

[sbarfurth]

Considering I'm not formally part of the project I can't give you the opportunity, but you are always free to give us your ideas and implementations.

About your points:

• There is no contract for modifiers, if you can implement a global modifier properly please go ahead and it will be evaluated
• While I agree some and every is a good use-case for global modifiers, I believe it might also make sense to apply them to specific rules sometimes

[adityasharat]

Cool, let me see what I can whip up.

[imbrn]

Hey everybody, this feature looks very interesting indeed.

The syntax suggested by @sebastianbarfurth sounds a little better for me. I like the idea to make validations for working upon arrays in some cases:

v8n()
  .array()
  .length(4)
  .every.positive()
  .test([1, 2, 3, -2]); // false

This is going to save us from having to write a lot of array specific validations, and it will give us so much flexibility. Actually, the includes syntax we already have implemented could be a synonym for:

v8n()
  .some.exact(5)     // .includes(5)
  .test([1, 2, 3, 4, 5]); // true

For this behavior to be achieved we don't need to change how modifiers are supposed to work right now. I think we should keep them tied to the next rule.

But the problem is that today we are passing the 'invert' argument to the Rule constructor. Maybe we need to change it to receive a collection of modifiers instead. And this is going to break the API though.

[adityasharat]

Yes, I agree the syntax suggested by @sebastianbarfurth is better.

Based on my reading of the code the use of invert:

  test(value) {
    return this.chain.every(rule => {
      try {
        return rule.fn(value) !== rule.invert;
      } catch (ex) {
        return rule.invert;
      }
    });
  }

is tightly coupled to the implementation of core.test. In other words, the implementation of modifiers is not sufficiently abstracted to allow new modifiers to be cleanly implemented or used.

In order to allow core.test to change behavior; i.e. iterate over the argument passed instead of testing the value against the chain of rules in this.chain there needs to be some property or state variable which applies to the entire validation, not just one rule.

Check out a reference implementation here for a clarity.

Speaking out aloud:

• Any reason against global modifiers?
• What if some and every are not implemented or called modifiers. How about something like this?

v8n()
  .array()
  .some()
  .number()
  .even()
  .test([1, 2, 3]); // true

[imbrn]

For now, let's keep modifiers for rules only, without globals. It looks more clear to me when we are declaring validation strategies.

I agree with you that the .not modifier is coupled with the core logic, and we should fix it.

What we can do is to move the modifiers logic into the Rule class. Instead of storing an invert property we can store an array of modifiers references which we should be applied to that Rule when performing the validation.

[adityasharat]

Sounds good. I will implement a draft where every Rule can have a collection of modifiers. While at it let me see how to refactor the implementation of core.test to use said modifiers more generically.

[imbrn]

Maybe you can move the test logic into the Rule class, and make it reduce over the modifiers. So you can call the Rule test function in each core validation function.

[adityasharat]

Interesting, I will evaluate that.

[imbrn]

@adityasharat are you making progress on this feature? This is a tricky one, and it requires a lot of core refactoring.

[adityasharat]

I was planning to work on this on Monday. I will publish a PR by Monday EOD if you want to see some progress.

Definitely a little tricky, but I have some ideas.

[imbrn]

@adityasharat Actually, I was playing around with this feature trying to get a better understanding of the problem context, and I end up with a complete implementation. I'm not sure if this is the ideal implementation, but I can make a PR and you maybe can help me to validate that?

[adityasharat]

Wow, sure man! Please go ahead, I would love to see the implementation.

[imbrn]

I made the PR at #61

[adityasharat]

This is pretty much in line with what I was planning. Awesome! Would love to see this change in the next version.

[imbrn]

Yes, this is going to be in 1.1.0 😁