CannerCMS/cannercms

Support async validation in field level

Mosoc opened this issue · 4 comments

Mosoc commented

Is your feature request related to a problem? Please describe.

It's a wide-used feature for checking username availability, filtering forbidden words in the content, or any other computing/request which need to take a certain amount of run time.

Describe the solution you'd like
It seems that making validation.validator perform not only synchronous function call but also asynchronous one with Promise is Intuitive way.

How to implement
The quick way in babel era is use async/await syntax.

validate = async () => {
 // ...
  let validatorResult = null;
  if(validator && isFunction(validator)) {
    validatorResult = await validator(value, reject);
  }
  // ...
}

But I think there are still some compatibility or performance issues.

For Promise only case, I prefer the method in Formik source code and doc, which Promisify either async function or normal one.

  runSingleFieldLevelValidation = (
    field: string,
    value: void | string
  ): Promise<string> => {
    return new Promise(resolve =>
      resolve(this.fields[field].props.validate(value))
    ).then(x => x, e => e);
  };

As this way, we can Promisify all the validate handle include orginal ajv validation.
Then, we can use Promise.all and map reduce to handle results and errors clearly.

After the discussion with Mosoc in Mosoc#1. Here are the summary:

  • We will remove the callback argument in a validator function.
  • If the validator is not a function, the value is invalid, and we log the error in the console
  • We will Promisify the validator function with Formik way
  • the resolved value of promised validator is may be string(invalid), Error(invalid), or others(valid)
  • Please ensure your test cases have the following situation
    • sync validator, return string → invalid
    • sync validator, throw Error → invalid
    • sync validator, unexpected Error → invalid
    • sync validator, no return, → valid
    • async validator, async/await, return string → invalid
    • async validator, async/await, throw Error → invalid
    • async validator, async/await, no return → valid
    • async validator, return Promise<string> → invalid
    • async validator, return Promise<Error> → invalid
    • async validator, return Promise<void> → valid
    • validator is not a function → invalid
  • the better solution to handle the Error is that we provide a ValidationError for users to make different behavior like logging the error stack of an unexpected Error in the console, we think we can implement this later, the current use cases above are good enough for now.
Mosoc commented

@abz53378
I checked the list of situations with some suggestions. Please take a look.

First, it should be valid when using async validator, with async/await syntax, and no return value in definition.

Because the function with async syntax always return promise, even It don't have return value.

const asyncExample = async () => {}
console.log(asyncExample())
> Promise {<resolved>: undefined}

async in console

The resolved value actually fits others(valid)
So that should be: async validator, async/await, no return → valid

Also, I want to check those condition statement:

  • async validator, return Promise (Throw error) → invalid
  • async validator, return Promise (Unexpected error caught) → invalid
  • async validator, return Promise (With no return, void return ) → valid

And finally, for condition: if validator is not a function, maybe we can block most cases of this condition happening by type checking in compile time.

  • validator is defined, but not a function → compile error
  • validator is undefined → skip customized validator section

@Mosoc, you are right, the case you mentioned should be valid, and I already edited my comment.
And, it's a good point that we should put this error in the other periods, and we prefer dealing with this in the build time(running webpack, see wiki), but currently the part is not implemented. So maybe we can deal with the error in this HOC temporarily?

Mosoc commented

Okay. As workaround, make it throw error when validator is not a function in runtime .

if(!isFunction(validator)) {
  throw 'Validator should be a function'
}