API changes for v1

[vitkon]

Discussion on API

API for form-container can be simplified, especially for validators.
Thoughts?

[mattdell]

Yeah, this was actually raised by @pereznieto with me earlier around the validators.

Care to explain the buildValidator method you showed me Fernando?

[pereznieto]

I've thought of an approach where the user can define validator functions that return booleans, rather than as is currently (returning an array with the validator function and the error message).

This approach simplifies the declaration of custom validators by giving users a buildValidator function which takes care of parsing simple validators into the format that form-container needs.

The way I've used it, buildValidator also gives an optional parameter called shouldValidate. This is a function of type (model: any) => boolean, and if passed, will only call the validator when it returns true. This is useful for conditionally validating certain fields, based on the values of other fields in the form.

Let me show you how I've used this in our project.

Previously, we defined validators like so:

export const isRequired = (prop: any, errorMessage = errorMessages.isRequired) => [
  (model: any) => !!model[prop],
  { [prop]: errorMessage },
];

export const isNumber = (prop: any, errorMessage = errorMessages.isNumber) => [
  (model: any) => (model[prop] && (!isNaN(Number(model[prop])) && Number(model[prop]) >= 0)),
  { [prop]: errorMessage },
];

export const isLessThanTenMillion = (prop: any, errorMessage = errorMessages.isLessThanOneMillion) => [
  (model: any) => Number(model[prop]) < 10000000,
  { [prop]: errorMessage },
];

I propose to give the user something like this:

// Somewhere on form-container
import { mapValues } from 'lodash';

export const buildValidator = (validator: (p: string, m: any) => boolean, error = 'Please enter a valid value') =>
  (prop: string, shouldValidate?: (model: any) => boolean) =>
  [
    (model: any) => (shouldValidate && !shouldValidate(model)) || validator(prop, model),
    { [prop]: error },
  ];

export const buildValidators = (validators: { [string]: (p: string, m: any) => boolean; }[], errors: string[]) => mapValues(
  validators,
  (validator: (p: string, m: any) => boolean, key: string) =>
  (prop: string, shouldValidate?: (model: any) => boolean, error = errors[key] || 'Please enter a valid value') =>
    [
    (model: any) => (shouldValidate && !shouldValidate(model)) || validator(prop, model),
    { [prop]: error },
    ]
);

Which will allow them to define validators like so:

// User's customValidators – individual validators
import { buildValidator } from 'form-container'; 

export const isRequired = buildValidator(
  (p: string, m: any) => !!m[p],
  'This field is required',
);

export const isNumber = buildValidator(
  (p: string, m: any) => (m[p] && (!isNaN(Number(m[p])) && Number(m[p]) >= 0)),
  'Please enter a valid number',
);

export const isInteger = buildValidator(
  (p: string, m: any) => (m[p] && (!isNaN(Number(m[p])) && Number(m[p]) % 1 === 0)),
  'Please enter a whole number',
);

export const isMoreThan0 = buildValidator(
  (p: string, m: any) => Number(m[p]) > 0,
  'Value must be over £0',
);

export const isEighteenOrMore = buildValidator(
  (p: string, m: any) => Number(m[p]) >= 18,
  'You must be 18 or older',
);

export const isEightyOrLess = buildValidator(
  (p: string, m: any) => Number(m[p]) <= 80,
  'You must be 80 or younger',
);

export const isLessThanTenMillion = buildValidator(
  (p: string, m: any) => Number(m[p]) < 10000000,
  'Please enter a number under £10,000,000',
);


// User's customValidators – all as one object
import { buildValidators } from 'form-container';

const rawValidators = {
  isRequired: (p: string, m: any) => !!m[p],
  isNumber: (p: string, m: any) => (m[p] && (!isNaN(Number(m[p])) && Number(m[p]) >= 0)),
  isInteger: (p: string, m: any) => (m[p] && (!isNaN(Number(m[p])) && Number(m[p]) % 1 === 0)),
  isMoreThan0: (p: string, m: any) => Number(m[p]) > 0,
  isEighteenOrMore: (p: string, m: any) => Number(m[p]) >= 18,
  isEightyOrLess: (p: string, m: any) => Number(m[p]) <= 80,
  isLessThanTenMillion: (p: string, m: any) => Number(m[p]) < 10000000,
};

// This object can come from somewhere else if the app's content is managed externally
const errorMessages = {
  isRequired: 'This field is required',
  isNumber: 'Please enter a valid number',
  isInteger: 'Please enter a whole number',
  isMoreThan0: 'Value must be over £0',
  isEighteenOrMore: 'You must be 18 or older',
  isEightyOrLess: 'You must be 80 or younger',
  isLessThanTenMillion: 'Please enter a number under £10,000,000',
};

const validators = buildValidators(rawValidators, errorMessages);

export default validators;

The way these validators are called/connected to the form is the same as current, save for the addition of the optional shouldValidate parameter.

It looks like this:

// components/Form.tsx
import * as React from 'react';
import { connectForm, IFormProps } from 'form-container';
import { 
  isRequired,
  isNumber,
  isInteger,
  isMoreThan0,
  isEighteenOrMore,
  isEightyOrLess,
  isLessThanTenMillion,
} from './customValidators';

interface IProps extends IFormProps {}

// arbitrary form component
export class Form extends React.PureComponent<IProps, {}> {
  render() {
    const { validationErrors, touched } = this.props.form;
    const { bindInput } = this.props.formMethods;
    return (
      <form>
        <div>
          <label>
            Test:
            <input
              {/* this is how you bind input to a form-container */}
              {...bindInput('test1')}
            />
            <small>{touched.test1 && validationErrors.test1}</small>
          </label>
          <label>
            Test:
            <input
              {/* this is how you bind input to a form-container */}
              {...bindInput('test2')}
            />
            <small>{touched.test2 && validationErrors.test2}</small>
          </label>
          <label>
            Test:
            <input
              {/* this is how you bind input to a form-container */}
              {...bindInput('test3')}
            />
            <small>{touched.test3 && validationErrors.test3}</small>
          </label>
        </div>
      </form>
    );
  }
}

// all validators for the form
const validators = [
  isRequired('test1'),
  isNumber('test1'),
  isInteger('test1'),
  isMoreThan0('test2'),
  isEighteenOrMore('test2'),

  // will only run if test1 has a value
  isEightyOrLess('test2', (model: any) => !!model.test1),

  // will only run if value of test2 equals 'Yes'
  isLessThanTenMillion('test3', (model: any) => model.test2 === 'Yes'),
];

// attaching our Form to form-container with validation
export default connectForm(validators)(Form);

I've added two options, one where the user defines validators individually, and one where they are all defined as an object. What I propose is to make buildValidator and buildValidators as shown above part of the form-container code to make them available for all users.

Let me know what you guys think. Happy to see if there's a better or more efficient way of doing this – my examples are what we've come up with in our current implementation of form-container.

[vitkon]

Let me play around with it, and we'll decide what's the best way forward with it

[mattdell]

This might be more of a feature request, but an issue we are encountering is that we only have the concept of an error with our validators and not the concept of a warning. Other form libraries have this so we might look to add the same.

In terms of implementation I think we're going to need to introduce a breaking change which will remove validators and put it into config.

I'm thinking instead of

const validators = [
  isRequired('firstName'),
];

const config: IFormConfig = {
  middleware: model => model,
};

export default connectForm(validators, config)(CreateClient);

We would have

const validatorsForErrors = [
  isRequired('firstName'),
];

const validatorsForWarningss = [
  isCapitalized('firstName'),
];

const config: IFormConfig = {
  middleware: model => model,
  errors: validatorsForErrors,
  warnings: validatorsForWarnings,
};

export default connectForm(config)(CreateClient);

In any case it should be extensible and validators should live inside a config object.

[vitkon]

can you add a feature request and explain in a bit more detail what is the use case for warning?
Never cam across it before

[mattdell]

Raised in #33.

Suggestion still stands about the suggestion of having one config parameter instead of two parameters.

[stephenwil]

+1 for the discussion, and to add, we may want to have validators that only apply when certain fields are conditionally rendered, and these conditions may come from props, so ideally want to have a way that either different sets of validators can be built up, or alternatively, where a validator has visibility of the model, can it have visibility of the props too, so the conditional validation can be applied at the validator level.

Right now, because the validators are applied to the form outside of the rendering component that has the props, we can't do such things.

[mattdell]

@stephenwil great idea but I believe this is already catered for in v0.1. You just need to send a second parameter to your ValidationRule validation function like so:

const isNotEqualToFoo = ValidationRuleFactory((value: any, model: any) => value !== model.foo, 'Value cannot be equal to foo');

[vitkon]

I would call second param allProps as form's model is a subset of allProps.
Otherwise it should give access to arbitrary props in your validator

[vitkon]

@pereznieto is you approach similar to ValidationRuleFactory that we have in validators.ts?
I like shouldValidate function!