Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions dumi/docs/concepts/state/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ export interface IState<V> {
activated: boolean
/** Current validate status. */
validateStatus: ValidateStatus
/** The error info of validation, ErrorObject type will be filled with ErrorObject.message. */
/** The error info of validation. */
error: ValidationError
/** The state's own error info, regardless of child states, ErrorObject type will be filled with ErrorObject.message. */
/** The state's own error info, regardless of child states. */
ownError: ValidationError
/** Ihe state's own error info, regardless of child states. */
/** The state's own error info, includes ValidationErrorObject error type, regardless of child states. */
rawError: ValidationRawError
/** Append validator(s). */
withValidator(...validators: Array<Validator<V>>): this
Expand Down Expand Up @@ -101,7 +101,7 @@ States will not be auto-validated until it becomes **activated**. And they will

### Raw Error

The state's own raw error info, regardless of child states. The difference compared to `ownError` is that it contains the type of `ErrorObject`. You can check details about them in issue [#82](https://github.com/qiniu/formstate-x/issues/82).
The state's own raw error info, regardless of child states. The difference compared to `ownError` is that it contains the type of `ValidationErrorObject`. You can check details about them in issue [#82](https://github.com/qiniu/formstate-x/issues/82).

### Disable State

Expand Down
4 changes: 2 additions & 2 deletions dumi/docs/concepts/validator/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ export type ValidationResult =
| null
| undefined
| false
| ErrorObject
| ValidationErrorObject

/** Object type validation result. */
export type ErrorObject = { message: string }
export type ValidationErrorObject = { message: string }

/** Return value of validator. */
export type ValidatorReturned =
Expand Down
28 changes: 19 additions & 9 deletions src/adapter/v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,13 @@ import * as v2 from 'formstate-x-v2'
import { BaseState } from '../state'
import * as v3 from '..'
import Disposable from '../disposable'
import { isPromiseLike, normalizeError, normalizeRawError } from '../utils'

interface IV3StateFromV2<T extends v2.ComposibleValidatable<unknown, V>, V> extends v3.IState<V> {
/** The original ([email protected]) state */
$: T
}

type ExcludeErrorObject<V> = V extends v3.ErrorObject ? never : V extends Promise<infer R> ? Promise<ExcludeErrorObject<R>> : V

type Conditional<K> = K extends any ? ExcludeErrorObject<K> : never

// Omit ErrorObject
type LegacyValidator<T> = (V: T) => Conditional<ReturnType<v3.Validator<T>>>

class Upgrader<T extends v2.ComposibleValidatable<unknown, V>, V> extends BaseState implements IV3StateFromV2<T, V> {
constructor(private stateV2: T) {
super()
Expand Down Expand Up @@ -50,12 +44,12 @@ class Upgrader<T extends v2.ComposibleValidatable<unknown, V>, V> extends BaseSt
setForV2(this.stateV2, value)
}
reset() { this.stateV2.reset() }
withValidator(...validators: Array<LegacyValidator<V>>) {
withValidator(...validators: Array<v3.Validator<V>>) {
if (
isV2FieldState(this.stateV2)
|| isV2FormState(this.stateV2)
) {
this.stateV2.validators(...validators)
this.stateV2.validators(...portV2Validators(...validators))
return this
}
throwNotSupported()
Expand All @@ -72,6 +66,22 @@ class Upgrader<T extends v2.ComposibleValidatable<unknown, V>, V> extends BaseSt
}
}

function portV2Validators<V>(...validators: Array<v3.Validator<V>>): Array<v2.Validator<V>> {
const normalizeRet = (v: any) => (
normalizeError(normalizeRawError(v))
)
return validators.map(validator => {
return (value: V) => {
const returned = validator(value)
if (isPromiseLike(returned)) {
return returned.then(normalizeRet)
} else {
return normalizeRet(returned)
}
}
})
}

/** Converts [email protected] state to [email protected] state */
export function fromV2<T extends v2.ComposibleValidatable<unknown, unknown>>(stateV2: T): IV3StateFromV2<T, T['value']> {
return new Upgrader(stateV2)
Expand Down
4 changes: 2 additions & 2 deletions src/debouncedState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { action, computed, makeObservable, observable, override, reaction } from
import { FieldState } from './fieldState'
import { ValidatableState } from './state'
import { IState, ValidateStatus, ValueOf } from './types'
import { debounce, isErrorObject } from './utils'
import { debounce, normalizeError } from './utils'

const defaultDelay = 200 // ms

Expand Down Expand Up @@ -52,7 +52,7 @@ export class DebouncedState<S extends IState<V>, V = ValueOf<S>> extends Validat

@override override get ownError() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在实现 DebouncedState 的时候,也应该去通过 override rawError 来实现,而不是 override ownError

#87 (comment) 这里所述,ownError = normalizeError(rawError) 这个逻辑对于 DebouncedState 也应该是成立的?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没注意这里有个 override,我看怎么改下。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

031f8a1
done

if (this.disabled) return undefined
if (this._error) return isErrorObject(this._error) ? this._error.message : this._error
if (this._error) return normalizeError(this._error)
return this.original.ownError
}

Expand Down
6 changes: 3 additions & 3 deletions src/state.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { action, autorun, computed, makeObservable, observable, when } from 'mobx'
import { ValidationRawError, ValidationError, IState, Validation, ValidateResult, ValidateStatus, Validator } from './types'
import Disposable from './disposable'
import { applyValidators, isValid, isPromiseLike, isErrorObject } from './utils'
import { applyValidators, isPromiseLike, normalizeRawError, normalizeError } from './utils'

/** Extraction for some basic features of State */
export abstract class BaseState extends Disposable implements Pick<
Expand Down Expand Up @@ -63,7 +63,7 @@ export abstract class ValidatableState<V> extends BaseState implements IState<V>
}

@computed get ownError() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是不是能挪到 BaseState 上了啊?因为看起来不管是什么样的 state,ownError = normalizeError(rawError) 这个逻辑都是成立的

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不太明确 BaseState 的定位,我看 BaseState 上一些属性,跟 ValidatableState 关系也挺大的

Copy link
Collaborator

@nighca nighca May 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BaseState 抽的是对所有 state 都成立的逻辑,现在 formstate-x 代码中所有对 interface IState 的实现都直接或间接地继承了它

ValidatableState 是校验逻辑的抽象,适用于那些有自己的校验逻辑/过程的 state

举个例子,TransformedState 继承了 BaseState 而没有继承 ValidatableState,是因为它没有“自己的校验逻辑/过程”,而是依赖的其包裹的 state($)的校验;TransformedState 不需要维护自己的 validators,也没有自己的 validate status,也不需要自己去维护一份单独的 error 信息

Copy link
Collaborator

@nighca nighca May 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BaseState 抽的是对所有 state 都成立的逻辑

基于这个标准看的话,现在 hasError 放在这上边就不太合适了,因为现在 hasError 的实现逻辑(hasError = !isPassed(rawError))并不是对所有 state 都成立的

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

假如我们有 packaged raw error,那么我们其实是能实现出这样一份对所有 state 都适用的逻辑的:

ownError = normalizeError(rawError)
error = normalizeError(packagedRawError)
hasError = !isPassed(packagedRawError)

这样不同的 state,它们去实现各自的 rawError & packagedRawError 就好,逻辑会相对清晰一点

不过现在我们没有 packagedRawError,也没有一个可靠的从 rawError 推出 hasError 的逻辑,所以那些 override 了 error 的地方都需要再去 override 一遍 hasError;基于这个前提,我觉得 #87 (comment) 这里提到的

不过感觉,它(hasError)的值也从 rawError / validationResult 直接得到要比从 error 直接得到更好

就不太对了;既然我们已经通过 throw 确保了 error object 的 message 一定不是空字符串,那么 hasError = !!error 的逻辑就还成立(而且是对所有的 state 成立),那走这个逻辑会对实现更方便?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

既然我们已经通过 throw 确保了 error object 的 message 一定不是空字符串,那么 hasError = !!error 的逻辑就还成立(而且是对所有的 state 成立),那走这个逻辑会对实现更方便?

#87 (comment)

我之前在这里就说了啊..,现在要把实现改回去吗?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#87 (comment)

我之前在这里就说了啊..

那边我没看懂你的意思..

现在要把实现改回去吗?

我倾向改回去,如果确实 hasError = !!error 这个逻辑还成立,而且基于这个逻辑确实更方便的话

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ba4b45e
done

return this.rawError ? (isErrorObject(this.rawError) ? this.rawError.message : this.rawError) : undefined
return normalizeError(this.rawError)
}

@computed get error() {
Expand Down Expand Up @@ -108,7 +108,7 @@ export abstract class ValidatableState<V> extends BaseState implements IState<V>
action('end-validation', () => {
this.validation = undefined
this._validateStatus = ValidateStatus.Validated
this.setError(isValid(result) ? undefined : result)
this.setError(normalizeRawError(result))
})()
}

Expand Down
12 changes: 6 additions & 6 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export type ValidationResult =
| null
| undefined
| false
| ErrorObject
| ValidationErrorObject

/** Return value of validator. */
export type ValidatorReturned =
Expand All @@ -19,9 +19,9 @@ export type Validation<TValue> = {
returned: ValidatorReturned // result of applying validators
}

export type ErrorObject = { message: string }
export type ValidationErrorObject = { message: string }
export type ValidationError = string | undefined
export type ValidationRawError = ErrorObject | ValidationError
export type ValidationRawError = ValidationErrorObject | ValidationError

export type ValidateResultWithError = { hasError: true, rawError: ValidationRawError, error: NonNullable<ValidationError> }
export type ValidateResultWithValue<T> = { hasError: false, value: T }
Expand All @@ -33,13 +33,13 @@ export interface IState<V = unknown> {
value: V
/** If value has been touched. */
touched: boolean
/** The error info of validation, ErrorObject type will be filled with ErrorObject.message. */
/** The error info of validation. */
error: ValidationError
/** If the state contains error. */
hasError: boolean
/** The state's own error info, regardless of child states, ErrorObject type will be filled with ErrorObject.message. */
/** The state's own error info, regardless of child states. */
ownError: ValidationError
/** Ihe state's own error info, regardless of child states. */
/** Ihe state's own error info, includes ValidationErrorObject error type, regardless of child states. */
rawError: ValidationRawError
/** If the state contains its own error info. */
hasOwnError: boolean
Expand Down
49 changes: 45 additions & 4 deletions src/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
import { observable } from 'mobx'
import { asyncResultsAnd, isValid, isArrayLike, isErrorObject } from './utils'
import { asyncResultsAnd, isValidationPassed, isArrayLike, isErrorObject, normalizeRawError, normalizeError } from './utils'
import { delayValue as delay } from './testUtils'
import { ValidationErrorObject } from './types'

describe('asyncResultsAnd', () => {
it('should work well with empty results', async () => {
const result = await asyncResultsAnd([])
expect(isValid(result)).toBe(true)
expect(isValidationPassed(result)).toBe(true)
})

it('should work well with all-passed results', async () => {
const result = await asyncResultsAnd([delay(null)])
expect(isValid(result)).toBe(true)
expect(isValidationPassed(result)).toBe(true)

await asyncResultsAnd([
delay(null, 30),
delay(undefined, 10),
delay(false, 20)
])
expect(isValid(result)).toBe(true)
expect(isValidationPassed(result)).toBe(true)
})

it('should work well with unpassed results', async () => {
Expand Down Expand Up @@ -99,5 +100,45 @@ describe('isErrorObject', () => {
expect(isErrorObject({message: 'msg'})).toBe(true)
expect(isErrorObject({message: 'msg', extra: 'ext' })).toBe(true)
expect(isErrorObject(new Error('error msg'))).toBe(true)

class Foo implements ValidationErrorObject { message = 'mewo' }
expect(isErrorObject(new Foo())).toBe(true)

class Bar extends Foo {}
expect(isErrorObject(new Bar())).toBe(true)
})
})

describe('normalizeValidationResult', () => {
it('normalizeRawError should work well', () => {
expect(normalizeRawError(false)).toBe(undefined)
expect(normalizeRawError(null)).toBe(undefined)
expect(normalizeRawError(undefined)).toBe(undefined)
expect(normalizeRawError('')).toBe(undefined)
expect(normalizeRawError('foo')).toBe('foo')
expect(normalizeRawError({ message: 'mewo' })).toEqual({ message: 'mewo' })

class Foo implements ValidationErrorObject { message = 'mewo' }
const foo = new Foo()
expect(normalizeRawError(foo)).toBe(foo)

class Bar extends Foo {}
const bar = new Bar()
expect(normalizeRawError(bar)).toBe(bar)
})

it('normalizeError should work well', () => {
expect(normalizeError(undefined)).toBe(undefined)
expect(normalizeError('')).toBe('')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这是符合预期的吗

expect(normalizeError('foo')).toBe('foo')
expect(normalizeError({ message: 'mewo' })).toBe('mewo')

class Foo implements ValidationErrorObject { message = 'mewo' }
const foo = new Foo()
expect(normalizeError(foo)).toBe('mewo')

class Bar extends Foo {}
const bar = new Bar()
expect(normalizeError(bar)).toBe('mewo')
})
})
33 changes: 26 additions & 7 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,36 @@
import { isObservableArray, IObservableArray } from 'mobx'
import { Validator, ValidationResult, ValidatorReturned, ErrorObject } from './types'
import { Validator, ValidationResult, ValidatorReturned, ValidationError, ValidationErrorObject, ValidationRawError } from './types'

export function isErrorObject(err: any): err is ErrorObject {
export function normalizeRawError(err: ValidationResult): ValidationRawError {
if (isErrorObject(err)) {
return err
}

if (err === false || err === '' || err === null) {
// TODO: print an alert?
return undefined
}

return err
}

export function normalizeError(rawError: ValidationRawError): ValidationError {
if (isErrorObject(rawError)) {
return rawError.message
}
return rawError
}

export function isErrorObject(err: any): err is ValidationErrorObject {
return err != null && typeof err === 'object' && 'message' in err
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里要对齐空字符串的行为吗?
就是类似这种 && err.message !== ''

}

export function isPromiseLike(arg: any): arg is Promise<any> {
return arg != null && typeof arg === 'object' && typeof arg.then === 'function'
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

: result is undefined 不对吧?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要注意下:

rawError: { message: '' }

的话,hasError 应该是 true 还是 false;如果是 true,src/state.ts 那里 class BaseStatehasError 逻辑也需要调整下,应该跟这里逻辑也是相关的

Copy link
Contributor Author

@Luncher Luncher May 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我感觉:
1、对于 validator 的返回值,falsy 表示没有错误
2、对于 hasErrorthis.error !== undefined 表示没有错误

这样是不是好点?

所以返回 { message: '' } 表示验证失败。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

所以 rawError: { message: '' } 的时候,认为没有错误(检查通过)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

所以 rawError: { message: '' } 的时候,认为没有错误(检查通过)?

我觉得不通过, 合理

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我打算限制一下这种情况,如果出现这种情况就直接给用户报错了,即:强制用户输入 message,这样能省不少事。不过在类型上不太好限制。 @nighca @lzfee0227 你们觉得咋样。

Copy link
Collaborator

@nighca nighca May 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

意思是,预期 ErrorObjectmessage 一定是一个非空字符串对吧?我没问题

不过在类型上不太好限制

这个问题不大,可以在文档里说

如果出现这种情况就直接给用户报错了

如果文档里明确说了的话,那不去报错也没啥问题,毕竟这不是一个容易犯的错误

Copy link
Contributor Author

@Luncher Luncher May 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不过感觉,它(hasError)的值也从 rawError / validationResult 直接得到要比从 error 直接得到更好

如果从 rawError/ validationResult 取的话。hasError 是不是得换个其他名字比较恰当,比如 hasRawError。当前这个 hasError 这么实现还有个好处是,它能同时表示 package errorown error,即:FormState/ArrayState/FieldState 等都能通过这个属性表示有没有错误。

而更接近上层的展示逻辑

hasErrorerror 对应,我们的 error 定位就是展示用的,比如 bindFormState 消费的就是这个 error,所以我觉得没问题的..

Copy link
Collaborator

@nighca nighca May 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.S. 以上我提到的“文档”,默认都是指广义的文档概念,包括 docs/ 中的内容,也包括代码中对字段、定义的注释说明(这些内容同样会被使用者阅读)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当前这个 hasError 这么实现还有个好处是,它能同时表示 package errorown error,即:FormState/ArrayState/FieldState 等都能通过这个属性表示有没有错误。

这个没太理解,因为现在 has error 表达的都是 has packaged error,而不是 has own error

hasErrorerror 对应,我们的 error 定位就是展示用的,比如 bindFormState 消费的就是这个 error,所以我觉得没问题的..

对,所以我现在感觉 hasErrorerror 更底层了;如果 error 是 error info 用于展示文案用途的“投影”,那么提供对应于这个投影的 has error 就意义不大了;用户更关心的应该是,是否存在错误(是否 pass),而不是是否存在一份用于展示的错误文案(尽管这俩在结果上很接近)

return normalizeRawError(result) === undefined
}

export function asyncResultsAnd(asyncResults: Array<Promise<ValidationResult>>): ValidatorReturned {
Expand All @@ -22,7 +41,7 @@ export function asyncResultsAnd(asyncResults: Array<Promise<ValidationResult>>):
let validResultCount = 0
asyncResults.forEach(asyncResult => asyncResult.then(result => {
// return error if any result is invalid
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIP: 注释的说法最好也调整下,“result valid” -> “validation passed”

if (!isValid(result)) {
if (!isValidationPassed(result)) {
resolve(result)
return
}
Expand Down Expand Up @@ -56,7 +75,7 @@ export function applyValidators<TValue>(value: TValue, validators: Validator<TVa
}

// 任一不通过,则不通过
if (!isValid(returned)) {
if (!isValidationPassed(returned)) {
return returned
}
}
Expand Down