Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
6 changes: 3 additions & 3 deletions dumi/docs/concepts/state/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ export interface IState<V> {
error: ValidationError
/** The state's own error info, regardless of child states. */
ownError: ValidationError
/** The state's own error info, includes ValidationErrorObject error type, regardless of child states. */
rawError: ValidationRawError
/** The state's validation result, is the same as the return value of the validator, regardless of child states. */
rawError: ValidationResult
/** Append validator(s). */
withValidator(...validators: Array<Validator<V>>): this
/** Fire a validation behavior imperatively. */
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 `ValidationErrorObject`. You can check details about them in issue [#82](https://github.com/qiniu/formstate-x/issues/82).
The state's validation result, is the same as the return value of the `validator`, 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).
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.

Nit: 后边有时间的时候,最好是在文档 Guide/Advanced 里补一下对应的 case(可以不在这个 PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK..


### Disable State

Expand Down
4 changes: 2 additions & 2 deletions src/adapter/v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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'
import { isPromiseLike, normalizeError } from '../utils'

interface IV3StateFromV2<T extends v2.ComposibleValidatable<unknown, V>, V> extends v3.IState<V> {
/** The original ([email protected]) state */
Expand Down Expand Up @@ -68,7 +68,7 @@ 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))
normalizeError(v)
)
return validators.map(validator => {
return (value: V) => {
Expand Down
14 changes: 10 additions & 4 deletions src/debouncedState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { action, computed, makeObservable, observable, override, reaction } from
import { FieldState } from './fieldState'
import { ValidatableState } from './state'
import { IState, ValidateStatus, ValueOf } from './types'
import { debounce, normalizeError } from './utils'
import { debounce, isPassed, normalizeError } from './utils'

const defaultDelay = 200 // ms

/** Infomation synced from original state */
type OriginalInfo<V> = Pick<IState<V>, 'activated' | 'touched' | 'error' | 'ownError'>
type OriginalInfo<V> = Pick<IState<V>, 'activated' | 'touched' | 'error' | 'ownError' | 'hasError'>

/**
* The state for debounce purpose.
Expand Down Expand Up @@ -42,7 +42,8 @@ export class DebouncedState<S extends IState<V>, V = ValueOf<S>> extends Validat
activated: this.$.activated,
touched: this.$.touched,
error: this.$.error,
ownError: this.$.ownError
ownError: this.$.ownError,
hasError: this.$.hasError
}
}

Expand All @@ -52,7 +53,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 normalizeError(this._error)
if (this.rawError) return normalizeError(this.rawError)
return this.original.ownError
}

Expand All @@ -62,6 +63,11 @@ export class DebouncedState<S extends IState<V>, V = ValueOf<S>> extends Validat
return this.original.error
}

@override override get hasError() {
if (this.disabled) return false
return !isPassed(this.rawError) || this.original.hasError
}

@override override get validateStatus() {
if (this.disabled) return ValidateStatus.WontValidate
if (
Expand Down
2 changes: 1 addition & 1 deletion src/fieldState.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ describe('FieldState', () => {
expect(state.hasError).toBe(false)
expect(state.error).toBe(undefined)
expect(state.ownError).toBe(undefined)
expect(state.rawError).toBe(undefined)
expect(state.rawError).toBe('')

state.setError('123')
expect(state.hasError).toBe(true)
Expand Down
4 changes: 2 additions & 2 deletions src/formState.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ describe('FormState (mode: object)', () => {
expect(state.hasError).toBe(false)
expect(state.error).toBe(undefined)
expect(state.ownError).toBe(undefined)
expect(state.rawError).toBe(undefined)
expect(state.rawError).toBe('')

state.setError('123')
expect(state.hasError).toBe(true)
Expand Down Expand Up @@ -1062,7 +1062,7 @@ describe('FormState (mode: array)', () => {
expect(state.hasError).toBe(false)
expect(state.error).toBe(undefined)
expect(state.ownError).toBe(undefined)
expect(state.rawError).toBe(undefined)
expect(state.rawError).toBe('')

state.setError('123')
expect(state.hasError).toBe(true)
Expand Down
16 changes: 16 additions & 0 deletions src/formState.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { observable, computed, isObservable, action, reaction, makeObservable, override } from 'mobx'
import { IState, ValidateStatus, ValidateResult, ValueOfStatesObject } from './types'
import { ValidatableState } from './state'
import { isPassed } from './utils'

abstract class AbstractFormState<T, V> extends ValidatableState<V> implements IState<V> {

Expand Down Expand Up @@ -47,6 +48,21 @@ abstract class AbstractFormState<T, V> extends ValidatableState<V> implements IS
}
}

@override override get hasError() {
if (this.disabled) {
return false
}
if (!isPassed(this.rawError)) {
return true
}
for (const state of this.childStates) {
if (state.hasError) {
return true
}
}
return false
}

/** If reference of child states has been touched. */
@observable protected ownTouched = false

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

/** Extraction for some basic features of State */
export abstract class BaseState extends Disposable implements Pick<
IState, 'ownError' | 'hasOwnError' | 'error' | 'hasError' | 'validateStatus' | 'validating' | 'validated'
> {
IState, 'rawError' | 'ownError' | 'hasOwnError' | 'hasError' | 'validateStatus' | 'validating' | 'validated'
Copy link
Collaborator

Choose a reason for hiding this comment

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

弱问这里的 'error' 不用留着吗?

> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

为啥这里加缩进?


abstract error: ValidationError
abstract rawError: ValidationResult
Copy link
Collaborator

Choose a reason for hiding this comment

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

rawErrorValidationResult 是对的吗?
后面还有类似的这种

Copy link
Contributor Author

@Luncher Luncher May 25, 2022

Choose a reason for hiding this comment

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

rawError 对 ValidationResult 是对的吗?

是预期的

后面还有类似的这种

啥?

Copy link
Collaborator

Choose a reason for hiding this comment

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

没啥,是预期的就好
我只是看到 error 对 result 感觉怪怪的


@computed get hasError() {
return this.error !== undefined
return !isPassed(this.rawError)
}

abstract ownError: ValidationError
Expand Down Expand Up @@ -54,9 +54,9 @@ export abstract class ValidatableState<V> extends BaseState implements IState<V>
@observable activated = false

/**
* The original error info of validation.
* The original return value of validation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个注释也改一下?

Suggested change
* The original return value of validation.
* The original validation result.

*/
@observable protected _error: ValidationRawError
@observable private _error: ValidationResult
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: 这个字段名改成 validationResult 会稍好点?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

其实我觉得 setError 跟 _error 配对挺好的,现在只改了这里一个, setError 暂时也动不了,是不是很奇怪?

Copy link
Collaborator

Choose a reason for hiding this comment

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

如果我们关于这一点是一致的:这里(_error & setError)叫 validation result 比叫 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.

甚至可以考虑:

// 这里是反映内在逻辑的
private validationResult: ValidationResult
private setValidationResult(v: ValidationResult) {
  this.validationResult = v
}

// 这里是 API 层面保持兼容的
public setError(v: ValidationResult) {
  return this.setValidationResult(v)
}

这样就比较清晰了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

如果这么改只是内部实现有这么一个概念,我感觉最主要的是让用户知道通过 setError 设置的值,跟 validation result 是等价的。
所以我觉得,在你上述的基础上把 setValidationResult 改成 public、把 setError 标识成 deprecated 会不会更合理一点

Copy link
Collaborator

Choose a reason for hiding this comment

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

setError 标识成 deprecated 会不会更合理一点

我觉得不用;你现在对用户侧的概念还是“error”(raw error 也是基于 error 的概念);所以在用户的感受上,setErrorrawError 应该是更像配对的;如果你把 setValidationResult 暴露出去,那你也应该把 rawError 命名调整成 validationResult


@computed get rawError() {
return this.disabled ? undefined : this._error
Expand All @@ -71,10 +71,10 @@ export abstract class ValidatableState<V> extends BaseState implements IState<V>
}

/**
* Set error info.
* Set validation result.
*/
@action setError(error: ValidationRawError) {
this._error = convertEmptyStringWithWarning(error)
@action setError(error: ValidationResult) {
this._error = error
}

/** List of validator functions. */
Expand Down Expand Up @@ -108,14 +108,14 @@ export abstract class ValidatableState<V> extends BaseState implements IState<V>
action('end-validation', () => {
this.validation = undefined
this._validateStatus = ValidateStatus.Validated
this.setError(normalizeRawError(result))
this.setError(result)
})()
}

@computed protected get validateResult(): ValidateResult<V> {
return (
this.error !== undefined
? { hasError: true, error: this.error } as const
this.hasError
? { hasError: true, error: this.error! } as const
: { hasError: false, value: this.value } as const
)
}
Expand Down
6 changes: 5 additions & 1 deletion src/transformedState.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { computed } from 'mobx'
import { computed, override } from 'mobx'
import { BaseState } from './state'
import { IState, Validator, ValueOf } from './types'

Expand Down Expand Up @@ -31,6 +31,10 @@ export class TransformedState<S extends IState<$V>, V, $V = ValueOf<S>> extends
return this.$.rawError
}

@override override get hasError() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: 这个 override 好像不需要,BaseState 提供了一份 hasError 实现:!isPassed(this.rawError),跟你这 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.

如果 transformoriginalState 是一个 AbstractFormState,就有可能不对了。

Copy link
Collaborator

Choose a reason for hiding this comment

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

哦对,我把 raw error 下意识理解成 packaged 了..

return this.$.hasError
}

@computed get error() {
return this.$.error
}
Expand Down
9 changes: 5 additions & 4 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ export type ValidationResult =
| false
| ValidationErrorObject

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

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

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

export type ValidateResultWithError = { hasError: true, error: NonNullable<ValidationError> }
export type ValidateResultWithValue<T> = { hasError: false, value: T }
Expand All @@ -39,8 +40,8 @@ export interface IState<V = unknown> {
hasError: boolean
/** The state's own error info, regardless of child states. */
ownError: ValidationError
/** Ihe state's own error info, includes ValidationErrorObject error type, regardless of child states. */
rawError: ValidationRawError
/** The state's validation result, is the same as the return value of the validator, regardless of child states. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

is the same as the return value of the validator

这个描述不准确?参考 https://qiniu.github.io/formstate-x/concepts/validator#validation-result

/** Result of validation. */
export type ValidationResult =
  string
  | null
  | undefined
  | false

/** Return value of validator. */
export type ValidatorReturned = 
  ValidationResult
  | Promise<ValidationResult>

/** A validator checks if given value is valid. **/
export type Validator<T> = (value: T) => ValidatorReturned

rawError: ValidationResult
/** If the state contains its own error info. */
hasOwnError: boolean
/** If activated (with auto-validation). */
Expand Down
27 changes: 5 additions & 22 deletions src/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import { observable } from 'mobx'
import { asyncResultsAnd, isValidationPassed, isArrayLike, isErrorObject, normalizeRawError, normalizeError, inValidErrorObjectMsg } from './utils'
import { asyncResultsAnd, isPassed, isArrayLike, isErrorObject, normalizeError, inValidErrorObjectMsg } 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(isValidationPassed(result)).toBe(true)
expect(isPassed(result)).toBe(true)
})

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

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

it('should work well with unpassed results', async () => {
Expand Down Expand Up @@ -109,25 +109,8 @@ describe('isErrorObject', () => {
expect(() => isErrorObject({ message: '' })).toThrow(inValidErrorObjectMsg)
})
})

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)
})

describe('normalizeValidationResult', () => {
it('normalizeError should work well', () => {
expect(normalizeError(undefined)).toBe(undefined)
expect(normalizeError('')).toBe(undefined)
Expand Down
Loading