Support for structured validation results

[Luncher]

Regarding field/form state In most cases, the verification result will be consumed in two ways. The first is to convert the verification result into a boolean form, that is, to obtain the hasError status by judging whether the error is empty, and then complete the logic control. The other is to pass a string type of error to the FormItem component for error prompting. In some cases, however, the application is expected to do more than just determine if there is an error or alert the user to an error. So I have some ideas as follows:

Validation results support structured information output, of course, the current implementation will be a default solution. Provide some auxiliary utils to facilitate the application code to consume the verification result.

I have a specific usage scenario here, Maybe There are better solutions to this problem. The above solution is one of the solutions we can think of. I threw out the problems and scenarios and wanted to hear everyone's suggestions.

[Luncher]

@nighca @lzfee0227 ~

[nighca]

I have a specific usage scenario here

@Luncher Within the example, seems that what u need is something like this:

class DomainBindingError extends Error {
  constructor(public domains: string[]) { super() }
  toString() {
    return `加速域名{formatDomains(bindOtherUserDomains)}已作为源站域名绑定在其他用户的存储空间上`
  }
}

function validateDomainBinding() {
  if (...) {
    // the validator returns an error object (which may produce an error message),
    // instead of an error message
    return new DomainBindingError(['abc.com'])
  }
}

// the state consumer can access both the error message and the original error object,
// so that it can produce a more detailed / interactive UI to display the error
state.error // "加速域名 ... 已作为源站域名绑定在其他用户的存储空间上"
state.rawError // [object DomainBindingError]
state.rawError.domains // ["abc.com"]

I'm not sure if it covers all the cases you supposed.

[Luncher]

Thank you for your kind reply.

I'm not sure if it covers all the cases you supposed.

My idea is that the validation function can throw more information if needed without the serialization step.

this solution you said seems like an elegant one. I have some rough ideas, but I haven't fully figured out how to implement them.

Needing to define a class for the validation result feels a bit annoying. And maybe toString is useless in some cases, just like javascript's toString. This way, however, enhances readability, and the validation result (structured) can be consumed by both the user and the program logic.

[nighca]

Thank you for your kind reply.

...

Needing to define a class for the validation result feels a bit annoying.

The error object must provide a way to produce its corresponding error message, so existing consumers of state work correctly. Custom class with toString is just an option to archive that, we can also change the code like this:

function validateDomainBinding() {
  if (...) {
    return {
      domains,
      toString: () => `加速域名{formatDomains(domains)}已作为源站域名绑定在其他用户的存储空间上`
    }
  }
}

state.rawError // { domains, toString }
state.rawError.domains // ["abc.com"]

For formstate-x, the work is almost the same with the previous example: just defining an interface ErrorObject, and deal with the error object:

interface ErrorObject {
  toString(): string
}

export type ValidationResult =
  string
  | null
  | undefined
  | false
  | ErrorObject

The developer who implements the validator will have to return an object which implements the interface ErrorObject. While the custom class way will provide better type-safety:

if (state.rawError instanceof DomainBindingError) {
  state.rawError.domains // ["abc.com"]
}

As a conclusion, formstate-x will not require developer to define a custom class and instantiate it. While it is a recommended way in Typescript projects.

[nighca]

The interface may also be like this:

interface ErrorObject {
  message: string
}

But that's very detail thing. We can save the discussion about it later.

[Luncher]

I will work on this.

[nighca]

@Luncher A proposal before PR will be ideal. So that we can reach agreement on the API in the first place.

[Luncher]

Background

the definition of ValidationResult constructs two types of value: string value and falsy value, string value use to imply that is error happened and serves the error message. the falsy value implies to is nothing not expected thing happens. users can be going on to do other ops.

the definition of ValidationResult

/** Result of validation. */

export type ValidationResult =
  string
  | null
  | undefined
  | false

utility function checks whether validation passed. the falsy value meaning passed.

/** If validation result is valid */
export function isValid(result: ValidationResult): result is '' | null | undefined | false {
  return !result
}

user use the derived state to check whether something is wrong, eg: hasError property.

the error is passed to the formItem component to display to the user.

function bindFormItem(state: FieldState<unknown>) {
  if (state.validating) return { hasFeedback: true, validateStatus: 'validating' } as const
  if (state.validated && !state.hasError) return { validateStatus: 'success' } as const
  if (state.validated && state.hasError) return { validateStatus: 'error', help: state.error } as const
}

validator produces an error string, formItem component consumes the message. productor and consumer are separated, and the component does not always consume the error string as directed. sometimes components need more information to figure out what happened. the practical example mentioned above using the serialize/deserialize overcome the problem. but I think is another way to solve this problem completely.

Proposal

Expend the definition

expend the definition of ValidationError and ValidationResult add structure type, inspired by @nighca :

interface ErrorObject {
  toString(): string
}

export type ValidationError = ErrorObject | string | undefined

export type ValidationResult =
  string
  | null
  | undefined
  | false
  | ErrorObject

the structure error type represents a literal object or class instance. sometimes new class instance may be preferred because it can utilize the instance of type guard.

Compatibility

the state.ownError property used by: binFormItem、 state.error、 validate result, assert it must be a string type, so we need to adjust it:

  function isErrorObject(err: ValidationError) {
    return err != null && typeof err === 'object' && 'toString' in err
  }

  @computed get ownError() {
     return this.disabled ? undefined :  isErrorObject(this._error) ? this._error.toString() :  this._error
  }

From the consumer's point of view:

the ErrorObejct is not a generic type as we need to export _error it is convenient for users to make the type guard.

  @computed get rawError() {
    return  this._error
  }

if (xx.rawError instanceof FooClass or isFooError(xx.rawError)) {
  // do something
  return (
      <Error>
        {xx.rawError.domains.map(it => <span>{it}</span>)}
      </Error>
  )
}

[nighca]

Actually I prefer interface { message: string } over interface { toString(): string }, for some reasons:

1. { message: string } is simpler, and serializable
2. { toString(): string } is too common: almost everything in JavaScript matches the interface. It is more likely for developers to make mistakes when returning error object, I made an example here

3. Consider an Error instance (new Error('Failed to fetch.')) is returned (which may be common), the value of its message ('Failed to fetch') is better than the result of toString()('Error: Failed to fetch').

   So { message: string } makes it easier for developers to just return an Error instance

[nighca]

export type ValidationError = ErrorObject | string | undefined

Maybe

export type ValidationError = string | undefined // for `state.error` & `state.ownError`
export type ValidationRawError = ErrorObject | string | undefined // for `state.rawError`

is better

[nighca]

And, if state.ownError stands for "own error" and state.error stands for "packaged error", do we need both "own raw error" and "packaged raw error"?

In the description above, you are implying that state.rawError stands for "own raw error". So what about "packaged raw error"?

[Luncher]

2. { toString(): string } is too common: almost everything in JavaScript matches the interface. It is more likely for developers to make mistakes when returning error object, I made an example here

you pointed out a very important point. I add a reference link about it sec-object.prototype.tostring.

[Luncher]

do we need both "own raw error" and "packaged raw error"?

From the perspective of fieldState, "error" and "packaged error" are the same. packaged makes more sense from a formState perspective. I think fieldState's "rawError" has no meaning for formState because rawError is more about implementing detail, unlike error(literal just for prompt.). but this introduces a bit of inconsistency. In conclusion, I don't think the packaged raw error is critical. I tend to leave it out for now and wait until there is a practical scenario.

[nighca]

In conclusion, I don't think the packaged raw error is critical

I agree.

While there is a small problem: the field name rawError does not imply that it stands for "own raw error" instead of "packaged raw error". Maybe we need to explain that in the field's doc at least.

[Luncher]

Maybe we need to explain that in the field's doc at least.

OK.