Add 'some' and 'every' as additional modifiers about v8n HOT 17 CLOSED

Interesting, I will evaluate that.

from v8n.

Yes, this is going to be in 1.1.0 😁

from v8n.

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.

from v8n.

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. 😃

from v8n.

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

from v8n.

Cool, let me see what I can whip up.

from v8n.

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.

from v8n.

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

from v8n.

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.

from v8n.

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.

from v8n.

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.

from v8n.

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

from v8n.

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.

from v8n.

@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?

from v8n.

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

from v8n.

I made the PR at #61

from v8n.

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

from v8n.