rsuite / schema-typed

Schema for data modeling & validation
MIT License
198 stars 28 forks source link

`addRule` Promise issue #60

Closed jspears closed 1 year ago

jspears commented 1 year ago

What version of schema-typed are you using?

2.0.3

Describe the Bug

The typescript type for MixedModel.addRule does not allow for promises to be returned from the onValid callback. It is not just a typing issue, as it is an API issue. The rules have no way of knowing if a function is async, until they are executed, because of this rules are entirely dependent on being called by the correct validator.


export type ValidCallbackType<V, D, E> = (
  value: V,
  data?: D,
  filedName?: string | string[]
) => CheckResult<E> | boolean;
...
export interface RuleType<V, D, E> {
  onValid: ValidCallbackType<V, D, E>;

Expected Behavior

The types would allow promises to be returned from the onValid function.

To Reproduce

The following from the documentation does not work in typescript.

const model = SchemaModel({
  username: StringType()
    .isRequired('This field required')
    .addRule((value:string):Promise<boolean> => {
      return new Promise(resolve => setTimeout(resolve, 500, value === 'abc'));
    }, 'Username already exists'),
});

I imagine the real fix is to add addAsyncRule method that accepts promise return types. This would of course be a breaking change.

The other 'fix' is to keep its existing api, and just ignore the async result when a validation that is async is called by the non-async validator. Something like:

export type ValidCallbackType<V, D, E> = (
  value: V,
  data?: D,
  filedName?: string | string[]
) => CheckResult<E> | boolean | Promise<boolean | CheckResult<E>>;
simonguo commented 1 year ago

Thanks, I also agree that adding an addAsyncRule is more reasonable from the API design. addRule can retain support for Promise, compatible with previous versions.

jspears commented 1 year ago

Fixed and merged, thanks!