vuejs / vue

This is the repo for Vue 2. For Vue 3, go to https://github.com/vuejs/core
http://v2.vuejs.org
MIT License
208.08k stars 33.69k forks source link

Custom error message from prop validator #9467

Open znck opened 5 years ago

znck commented 5 years ago

What problem does this feature solve?

Currently, if a custom validator fails, we get a console error log saying Invalid prop: custom validator check failed for prop 'email' which is not helpful if you're using a third-party component. The only way to find out what failed is to jump into the source code of the component and try to understand what does this custom validator do. If the custom validator can provide a custom message that immensely changes developer experience e.g. Instead of Invalid prop: custom validator check failed for prop 'email', it can say, Invalid prop: the prop 'email' should be a valid GMail address.

What does the proposed API look like?

No change in API signature only behavior of validator function. If a validator function throws an error, use it as a custom message for prop validation. Also, allow {{name}} interpolation in error message. So the email can be defined as:

...
  props: {
    email: {
      validator(value) {
        if (!value.endsWith('@gmail.com')) throw new Error('the prop '{{name}}' should be a valid GMail address.')
        return true
     }
   }
...
posva commented 5 years ago

Duplicate of #8726 tho 😆 I think we should talk about the api a bit first

znck commented 5 years ago

It was one-liner but I agree on finalizing the API first. With custom validation messages, we can have user-land packages shipping complex validation helpers. I, myself, can use it in znck/prop-types.

dietergeerts commented 5 years ago

FYI: At the moment, there is a way to have a better warning though, one where you can add the value passed to the prop in the warning message:

/**
 * @type {RegExp}
 */
const validatorWarningRegExp = /Invalid prop: custom validator check failed for prop "(.*?)"\./;

/**
 * Better warn messages than the built-ins (adds value for invalid prop, ...).
 *
 * @see {@link https://vuejs.org/v2/api/#warnHandler}
 *
 * @function
 * @param {VueConstructor} vue
 * @param {function(...*)} handler
 */
export const initGlobalWarnHandlerForVue = (vue, handler) => {
  vue.config.warnHandler = (message, vm, trace) => {
    const validatorWarning = validatorWarningRegExp.exec(message);
    if (validatorWarning) {
      handler('[APP] Vue:', message, '\nValue:', vm[validatorWarning[1]], trace);
    } else {
      handler('[APP] Vue:', message, trace);
    }
  };
};
chrisvfritz commented 5 years ago

When the returned value from a validator is truthy, I wonder if it would be simpler to check if what's returned is a string, then if it is, make that the warning message. Thoughts?

diachedelic commented 5 years ago

@chrisvfritz that would break validators that look like val => val.name, i personally expect only the truthiness of a validation result to be checked.

chrisvfritz commented 5 years ago

@diachedelic That's true. I like the thrown error better then. 🙂

@posva As for the API, I think it can be discussed separately if throwing an error could be an acceptable solution. Though my personal thought is that unless we're using this for something else in validators, I think it might be most intuitive to just bind props (and $props) directly to the function, so they can be accessed the same way as anywhere else in the component.

posva commented 5 years ago

But right now validation takes place in order so not all props are available to validate. I don't think validators should have access to this though

chrisvfritz commented 5 years ago

@posva Yes, the change would require postponing prop validation until after the first pass of processing props. Regarding validators having access to this, what are your concerns?

posva commented 5 years ago

It would allow people to test against anything related to the component and I don't think we should allow that because a validator should be a pure function. Apart from props and maybe slots presence, what would you use from this

chrisvfritz commented 5 years ago

@posva That's it - probably just props and maybe slots. I wouldn't expect the entire instance to be initialized at that point, similar to how not everything is available on this in beforeCreate. When you say "a validator should be a pure function", what are the disadvantages or edge cases you're trying to avoid?

posva commented 5 years ago

I want to avoid access to other properties, for example the $router, $store or other things.

Itee commented 5 years ago

This is exactly what is asked on the forum: Props validation error message override

So to resume my point of view:

We could/should be able to do things like this:

props: {
    foobar: {
        type: String,
        validator: ( value ) => { return [ 'foo', 'bar' ].includes( value ) },
        errorMessage: 'Invalid prop "foobar", available values are "foo", "bar".'
    }
}

and the incriminated code with one implementation to that purpose could be:

/**
 * Assert whether a prop is valid.
 */
function assertProp (
    prop,
    name,
    value,
    vm,
    absent
) {
    if (prop.required && absent) {
        if (prop.errorMessage) {
            warn(
                prop.errorMessage,
                vm
            );
        } else {
            warn(
                'Missing required prop: "' + name + '"',
                vm
            );
        }
        return
    }
    if (value == null && !prop.required) {
        return
    }
    var type = prop.type;
    var valid = !type || type === true;
    var expectedTypes = [];
    if (type) {
        if (!Array.isArray(type)) {
            type = [type];
        }
        for (var i = 0; i < type.length && !valid; i++) {
            var assertedType = assertType(value, type[i]);
            expectedTypes.push(assertedType.expectedType || '');
            valid = assertedType.valid;
        }
    }
    if (!valid) {
        if (prop.errorMessage) {
            warn(
                prop.errorMessage,
                vm
            );
        } else {
            warn(
                "Invalid prop: type check failed for prop \"" + name + "\"." +
                " Expected " + (expectedTypes.map(capitalize).join(', ')) +
                ", got " + (toRawType(value)) + ".",
                vm
            );
        }
        return
    }
    var validator = prop.validator;
    if (validator) {
        if (!validator(value)) {
            if (prop.errorMessage) {
                warn(
                    prop.errorMessage,
                    vm
                );
            } else {
                warn(
                    'Invalid prop: custom validator check failed for prop "' + name + '".',
                    vm
                );
            }
        }
    }
}

So what about a such implementation ?

nicooprat commented 5 years ago

I think the validator method should pass some kind of a reject function that let the developer throw an error according to the validation. Or the validator function could return true, false, or a string that will be displayed as the error message. The errorMessage property you propose is too limiting in my opinion.

rahicks26 commented 4 years ago

Why not let the validator return the error message? If result is a function call the function, this allows custom logging nicely, if it returns a string use it as the message. Otherwise return the default message.

tohagan commented 4 years ago

errorMessage is definitely too limiting. e.g. I'm coding composable run-time type validators for deep objects where I need to report a validation error on a nested value in a complex Object prop (I forked and adapted a lib for use in Vue).

@rahicks26 I agree that just returning a string as validation error would be preferred BUT sadly it's not backward compatible with the existing API so I think throwing an exception is the only option. Its also safer since your custom validator may itself have a bug that throws an exception.

aminimalanimal commented 4 years ago

This doubles the console messages, but you can currently just console.warn or console.error from within existing validators.

validator(value) {
  const isValid = !!<what you’d normally return from your validator>
  if (!isValid) {
    console.warn(`${value} is not valid`)
  }
  return isValid
}
antoniandre commented 3 years ago

The simplest way for now to print a custom error message and not produce a double error is to raise a console error (in my case I don't want to throw an Error) and return true in the validator function. Like in this example:

selectableRows: {
      validator: value => {
        if (![undefined, true, false, 1, '1', ''].includes(value)) {
          console.error(
            'Wrong value for the `selectableRows` prop. ' +
            `Given: "${value}", expected one of: [undefined, true, false, 1, '1', ''].`
          )
        }
        return true
      }
    }
Dawdre commented 2 years ago

The simplest way for now to print a custom error message and not produce a double error is to raise a console error (in my case I don't want to throw an Error) and return true in the validator function. Like in this example:

selectableRows: {
      validator: value => {
        if (![undefined, true, false, 1, '1', ''].includes(value)) {
          console.error(
            'Wrong value for the `selectableRows` prop. ' +
            `Given: "${value}", expected one of: [undefined, true, false, 1, '1', ''].`
          )
        }
        return true
      }
    }

Whilst this does work, it still doesn't feel like it should be the way to achieve this. Logging an error could be useful (in an isolated context within a custom validator) but I would guess most who are writing code that is going to production shouldn't be adding console.error in places.

My use case for needing custom error messages is for both cross prop validation and validation on itself. I am the author of a design system and therefore I am concerned about consumers/other developers being aware of prop validation when in development. I need to avoid errors to leak to anywhere but development (Vue warn won't do this).

I have had to create this which I am using in a props watch handler with immediate: true to be able to validate a component's props against each other:

export function validateVueProps(propCondition, errorMessage) {
  if (propCondition && process.env.NODE_ENV !== "production") {
    throw new TypeError(`${errorMessage}`);
  }
}

My second use case is having a custom validator for prop type of Array. I want to specify a value and label key that exists in the passed Array. It would be nice to display custom message - "Your options must contain a label and value", for example. A similar thing referenced here https://github.com/vuejs/vue/issues/6496#issuecomment-922364934. e.g.

validator: (options) => options.map(key => Object.keys(key)).includes("label" && "value")

A generic [Vue warn] on quite specific validation seems antithetical.

My specific use cases (I appreciate cross prop validation is not really on topic here) shouldn't necessarily deter from the idea that:

Just my thoughts, thanks all.

vinceramcesoliveros commented 2 years ago

Is this still in progress?

ddenev commented 2 years ago

+1