poteto / ember-changeset-validations

Validations for ember-changeset
http://bit.ly/ember-changeset-demo
Other
220 stars 98 forks source link

[rfc]: Externalize schema validation #342

Open snewcomer opened 2 years ago

snewcomer commented 2 years ago

RFC

One broad goal we have is to potentially deprecate the use of ember-changeset-validations. Having both ember-changeset and ember-changeset-validations presents some confusion. If we went forward, ember-changeset would be the single library we would use to validate our data.

Currently, at a high level, the changeset API for validations is rather unintuitive and verbose.

e.g. ember-changeset-validations README

This addon updates the changeset helper by taking in a validation map as a 2nd argument (instead of a validator function).

Do we need this? Here is an example of the migration.

import lookupValidator from 'ember-changeset-validations';

const EmployeeValidations = {
  email: validateFormat({ type: 'email' }),
  password: [
    validateLength({ min: 8 }),
    validatePasswordStrength({ minScore: 80 })
  ],
  passwordConfirmation: validateConfirmation({ on: 'password' })
};

const changeset = new Changeset(this.model, lookupValidator(EmployeeValidations), EmployeeValidations);

Nominally, this could look like the following...

let schema = yup.object().shape({
  email: yup.string().email().required(),
  password: yup.string().required().min(8),
  passwordConfirmation: yup.string()
     .oneOf([yup.ref('password'), null], 'Passwords must match')
});

const changeset = new Changeset(this.model, schema);

As long as schema adhered to the interface we defined (something like isValid and/or validate methods), then we can detect errors in the current in progress changeset and error appropriately.

https://github.com/jquense/yup

Upsides

Downsides

ref https://github.com/validated-changeset/validated-changeset/issues/166

snewcomer commented 2 years ago

One problem I'm seeing with yup is that is only returns one error for the schema

let schema = yup.object().shape({
  name: yup.string(),
  email: yup.string().email().required(),
  age: yup.number().min(18),
});
await schema.validate({ name: 'jimmy', age: 11 });

// ValidationError: age must be greater than or equal to 18

Table stakes would be all errors...for form rendering for example.

snewcomer commented 2 years ago

Well I guess validationAt would work if we iterate over each key. Just need to make sure our string key a position format is the same. However, this would require us to discover all the keys in question (might be arbitrarily nested, arrays, etc)

snewcomer commented 2 years ago

Ok yup abortEarly: false would give us errors. Would require user configuration.

Another option instead of internalizing validation and settings errors dependent on the result of validate, we could instead require the user to push in the errors. e.g. changeset.validate will call their ValidatorClass.validate function and give them errors they can specifically add to the changeset.

const errors = await changeset.validate();
errors.forEach((e) => changeset.addError(e));

That would give them the flexibility needed and decouples changeset from managing not only validations but creating errors as well.

snewcomer commented 2 years ago

https://github.com/validated-changeset/validated-changeset/issues/166#issuecomment-1073159453

Will provide a new changeset. Something like SchemaChangeset or ValidatedChangeset and avoid deprecating this package for now.