Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rfc]: Externalize schema validation #166

Open
snewcomer opened this issue Mar 12, 2022 · 1 comment
Open

[rfc]: Externalize schema validation #166

snewcomer opened this issue Mar 12, 2022 · 1 comment
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@snewcomer
Copy link
Collaborator

snewcomer commented Mar 12, 2022

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

  • Less confusion in the changeset ecosystem
  • Simpler API

Downsides

ref #166

@snewcomer
Copy link
Collaborator Author

That path forward seems to be providing a second changeset with this behavior. Bolting on or changing the API without knowing the consequences (would yup be flexible enough? arbitrary nested changes?) would be detrimental.

@SergeAstapov SergeAstapov added good first issue Good for newcomers and removed help wanted labels Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants