longshotlabs / simpl-schema

A JavaScript schema validation package that supports direct validation of MongoDB update modifier objects
https://www.npmjs.com/package/simpl-schema
MIT License
560 stars 114 forks source link

Difficulty getting validation errors for array items in template #237

Open coagmano opened 6 years ago

coagmano commented 6 years ago

Hi,

I've been trying to reactively validate an input field which contains a delimited string, which parses into an array of email addresses. (eg: foo@example.com, bar@example2.com)

I'm triggering the validity state on the input using a blaze helper that calls context.keyIsInvalid(key);

This works fine when the the field is empty as the key matches the field, but I'd like to use the email regex to check the parsed array items and feed that back into the validation state and messages.

Currently errors in the array are named field.$ (ie. field.0, field.1), and so they don't show up when using context.keyIsInvalid.

I didn't see another more suitable way to do this so I ended up filtering the validationErrors array:

errorMessage(key) {
  const inst = Template.instance();
  return inst.validationContext
    .validationErrors()
    .filter(error => error.name.startsWith(key))
    .map(error => inst.schema.messageForError(error))
    .join(', ');
},
isValid(key) {
  const vCtx = Template.instance().validationContext;
  return (
    vCtx.validationErrors().filter(e => e.name.startsWith(key)).length === 0
  );
}

Which feels a little painful

Is there a better way to do this? If not, is there interest in a reactive function that returns errors for array items under a top level key?

aldeed commented 6 years ago

@coagmano Can you post an example of the object you're validating and what .validationErrors() returns after you validate it?

coagmano commented 6 years ago

Sure, lets start with the validation flow The form is a text field where you enter emails delimited by comma

input.value === 'valid@email.com, notValid, another@example.org'

In response to an event, it's split, trimmed and saved as an array to a ReactiveDict

inst.formState.set('to', ['valid@email.com', 'notValid', 'another@example.org'])

That triggers a validation on the array

inst.validationContext.validate(inst.formState.get('to'), { keys: ['to'] })

Which triggers a re-run of helper

isInvalid(key) {
    const inst = Template.instance()
    return inst.validationContext.keyIsInvalid(key) 
}

Now my issue:

Calling context.keyIsInvalid('to') always returns true as long as there's more than 1 char in the input, because the value is an array and has more than 1 item.

For an array with ['valid@email.com', 'notValid', 'another@example.org'] calling context.validationErrors() returns:

[{name: "to.1", value: "notValid", ... }]

at which point:

context.keyIsInvalid('to') === false
context.keyIsInvalid('to.0') === false
context.keyIsInvalid('to.1') === true
context.keyIsInvalid('to.2') === false

Which is why I instead iterate over the errors and check if the name starts with 'to' to collect the errors from within the array.

Now this is all expected behaviour. What I'd like to see is being able to pass a wildcard for errors in an array into keyIsInvalid and similar helpers like so:

context.keyIsInvalid('to.$')

And possibly an array of keys, to include the array validation itself:

context.keyIsInvalid(['to', 'to.$'])

Which would save having to grab all errors and filter for the ones matching the array validation

The point of this issue is

  1. to see if there's a better way, OR
  2. to see if there's interest in adding this feature to the existing functions

I can put in the time to provide a PR if this is desired


Here's the relevant part of my schema:

new SimpleSchema({
  to: { type: Array, minCount: 1 },
  'to.$': SimpleSchema.RegEx.Email,
  // more keys
});
aldeed commented 6 years ago

OK thanks for describing. I can't think of a better way at the moment. If you're thinking of adding something like context.anyArrayItemIsInvalid('to') (false if any key matching to.$ has an error), I think that makes sense. You could do it as context.keyIsInvalid('to.$'), but it might not be as clear to people what that will do.

Note that keyIsInvalid already takes a second argument that is a generic key, but I don't remember why and it doesn't look like the code that is there would actually work properly with a generic key. https://github.com/aldeed/simple-schema-js/blob/master/package/lib/ValidationContext.js#L75

coagmano commented 6 years ago

I might just solve this in my own project with a global helper that checks for array errors then. Makes sense to wait if there's interest down the line before shoehorning this in

Thanks for taking the time to consider this!