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

return null when no errors #48

Open
smeijer opened this issue Jan 9, 2017 · 2 comments
Open

return null when no errors #48

smeijer opened this issue Jan 9, 2017 · 2 comments

Comments

@smeijer
Copy link

smeijer commented Jan 9, 2017

Why not return null when there are no validation errors?

That way we could do something like:

// Usage
const dogValidator = combineValidators({
  name: composeValidators(
    isRequired,
    isAlphabetic
  )('Name'),

  age: isNumeric('Age')
});

const dog = { name: 'Tucker', age: '10' };

// style 1
const errors = dogValidator(dog);
if (errors) {
  throw new ValidationError(errors);
}

db.save(dog);

// style 2
const errors = dogValidator(dog);
if (!errors)
  db.save(dog);
}

I'm aware of the assertion helpers, but in my opinion they shouldn't be really necessary.

@jfairbank
Copy link
Owner

Hi, @smeijer.

This isn't a bad idea. Out-of-the-box interop with redux-form is very important, and it seems to support a validate function that returns a falsy value. However, this would be a breaking change, so I'm hesitant to immediately change this. Maybe some opt-in behavior for now via an option until the next major release would be a good middle ground.

So something like this:

const dogValidator = combineValidators({
  name: composeValidators(
    isRequired,
    isAlphabetic
  )('Name'),

  age: isNumeric('Age')
}, { nullWhenValid: true });

@smeijer
Copy link
Author

smeijer commented Jan 15, 2017

You're totally right about the breaking change. Your solution sounds perfect to me.

The developers that depend on an empty object being present, can easily use the || operator to force this result. That's actually a pretty usual convention in the world of javascript. So it should feel familiar to them.

const errors = dogValidator(dog) || {}; // errors = {} when dogValidator returns null

Your solution of the extra option in the next release, and changing the default on the next major sounds awesome. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants