loopbackio / loopback-datasource-juggler

Connect Loopback to various Data Sources
http://www.loopback.io
Other
278 stars 366 forks source link

Model.validate does not forward attr and conf to the callback #1556

Closed tylkomat closed 6 years ago

tylkomat commented 6 years ago

Description/Steps to reproduce

Since the attribute is not forwarded to the custom validator there has to be one validator for each property although the validation might be the same. Custom validators should be as flexible as the build-in validators.

Link to reproduction sandbox

Expected result

Additional information

kjdelisle commented 6 years ago

@tylkomat I think I understand what you're asking for, but just to make sure, would you be able to give us a UX sample detailing what you're hoping to see here in terms of the forwarding to the callback?

I'm asking because I want to see if what you're proposing constitutes a breaking change from the API's perspective.

tylkomat commented 6 years ago

This is the code directly from validations.js

function validateCustom(attr, conf, err, options, done) {
  if (typeof options === 'function') {
    done = options;
    options = {};
  }
  conf.customValidator.call(this, err, done);
}

As you can see attr and conf are not forwarded to the call. These could be forwarded by appending to the call function like this: conf.customValidator.call(this, err, done, attr, conf); which would not result in any breaking changes, but would also not follow the general convention for callbacks, which is any way not followed in async case where usually the error should be given as first parameter to the done function. For example to achieve the same behaviour like the validateLength validator with different error messages I have to put the same configuration into the validate function and my custom validator function. For example:

User.validatesLengthOf('password', {min: 7, message: {min: 'too weak'}});

Is something like this when one wants to achieve the same behaviour:

function myLengthValidator(attr, conf) {
  return function(err) {
    ...
  };
}
const config = {min: 7, message: {min: 'too weak'}};
User.validate('password', myLengthValidator('password', config), config);

While it should be like this (to not break anything or another order of the parameters):

function myLengthValidator(err, done, attr, conf) {
  ...
}
const config = {min: 7, message: {min: 'too weak'}};
User.validate('password', myLengthValidator, config);
stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 6 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.