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

Supply all props to validator as second argument #6787

Closed
scottbedard opened this issue Oct 12, 2017 · 5 comments
Closed

Supply all props to validator as second argument #6787

scottbedard opened this issue Oct 12, 2017 · 5 comments

Comments

@scottbedard
Copy link

What problem does this feature solve?

Right now (2.4.4), props can be validated based on their value, but are unable to take other props into consideration. With access to a props object, we could do more complex validation.

For example, imagine a component with a foo and bar prop, where at least one of these props must be defined. Currently, this logic would have to be placed in the component's lifecycle hooks or render function. I would like the ability to keep it within the validator functions, as I think it's a more appropriate place for the logic.

What does the proposed API look like?

export default {
  props: {
    foo: {
      validator: (foo, props) => foo || props.bar
    },
    bar: {
      validator: (bar, props) => bar || props.foo
    }
  }
}
@scottbedard scottbedard changed the title Supply all props to validator as second argument [Feature request]: Supply all props to validator as second argument Oct 12, 2017
@posva
Copy link
Member

posva commented Oct 12, 2017

I gave this a shot and while it's possible to add I think it could introduce some inconsistent behaviors or complexify further the validation. For example, it's weird when used with a type because you expect both rules to validate, of course, you could say that the user is responsible for the type validation inside of the validator, we could even add a warning for that, but some people may want to use both at the same time. It's a bit similar with the required attribute. For example, at the moment, the validator only runs when the required field is set to true or a value is provided.
But maybe it's just that it's late and I'm tired 😆

@scottbedard
Copy link
Author

I'm not sure I follow your concern @posva. I don't see this feature effecting the current behavior of required or type. I could be missing something, but wouldn't they continue to behave as they currently do?

@posva
Copy link
Member

posva commented Oct 13, 2017

no, they wouldn't
Right now, a non required prop with no value never runs the validator which means that in order to support your usecase, we'll have to run the validator even when a value is not provided, which will show a warning (or throw an error depending of the validator) for other people using them to actually validate the information. Basically, I think validator is the wrong place to do this.
We could maybe support a function for required that gets passed all the props:

required: ({a, b}) => a || b

But this should be stripped off on production 🤔
edit: I'm not sur letting required be a function is a good idea

@scottbedard
Copy link
Author

Damn, I see your concern now... If one of two props is required, but neither are passed in, neither validator will ever run and no warning will be thrown. A dedicated function might be a better place for this, I'll keep brainstorming APIs. Thanks!

@yyx990803 yyx990803 changed the title [Feature request]: Supply all props to validator as second argument Supply all props to validator as second argument Nov 20, 2017
@vuejs vuejs deleted a comment from mirik-k Feb 10, 2018
@yyx990803
Copy link
Member

Closing since it seems we were not able to find a good solution for the concerns, and it is possible to implement the logic in lifecycle hooks (e.g. beforeUpdate).

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

3 participants