Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimize validate() behavior when validation disabled #75

Merged
merged 4 commits into from
Mar 7, 2022
Merged

Optimize validate() behavior when validation disabled #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd50532
optimize validate when state disabled
Luncher Mar 3, 2022
1985c26
update test case
Luncher Mar 3, 2022
27fab96
update validateResult
Luncher Mar 4, 2022
7f33e56
update comment
Luncher Mar 4, 2022
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
11 changes: 11 additions & 0 deletions src/fieldState.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,17 @@ describe('FieldState validation', () => {
expect(state.hasError).toBe(false)
expect(state.error).toBeUndefined()

runInAction(() => options.disabled = false)

await delay()
expect(state.validating).toBe(false)
expect(state.validated).toBe(false)
expect(state.hasError).toBe(false)
expect(state.error).toBeUndefined()

runInAction(() => options.disabled = true)

await delay()
state.onChange('123')
await delay()
state.onChange('')
Expand Down
21 changes: 16 additions & 5 deletions src/fieldState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@ export default class FieldState<TValue> extends Disposable implements Composible
return !!this.error
}

/**
* The most recent validation result.
* If state is disabled and later enabled, the result of the last validation is obtained.
* If you need to clear the validation result, please use the reset function.
*/
@computed private get validateResult(): ValidateResult<TValue> {
return this.hasError
? { hasError: true, error: this.error } as const
: { hasError: false, value: this.value } as const
}

/**
* If the validation has been done.
* It does not means validation passed.
Expand Down Expand Up @@ -152,6 +163,10 @@ export default class FieldState<TValue> extends Disposable implements Composible
* Fire a validation behavior.
*/
async validate(): Promise<ValidateResult<TValue>> {
if (this.validationDisabled) {
return this.validateResult
Copy link
Collaborator

@lzfee0227 lzfee0227 Mar 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

NIP: 我突然觉得 disabled 的定位应该重新明确一下…
除了看不见之外，还意味着什么？
比如“残留”着上次的状态以待后续潜在的重新 enabled 后接着用？
比如 disabled 的时候 set reset （以及后面的 touched）这些接口该如何理解？
比如一个 disabled 的 state 作为子 state 存在的时候，父 state 的 value 应该能拿到它的值吗？
诸如此类，等等

比如我就会想：
disabled 的时候要不要不清 error？
如果不清，那么转为 enabled 的那一刻，应该清了重新等 validate 或干脆不 active 吗？
如果都不清，那么 disable when 变更的心智模型就跟 validators 变更有区别了，这确实无所谓吗？

Copy link
Collaborator

@nighca nighca Mar 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

我们的表单状态模型是一棵 state 树，每个节点都是一个 state

在我自己的理解里，关于 disabled 的最直接的诉求是这样的：被 disabled 的 state，就像从当前 state 树上被摘掉了一样；原因很简单，因为这个 feature 对标的就是表单中的条件输入（conditional input）的场景，而这个场景对应的 input，就像是从当前 UI 树上被摘掉了一样

因此在理想的情况下

  1. 它的校验状态、结果不再反映到最终整体的校验状态、结果里
  2. 如果整个 state 树要做校验（validate()），它并不会随之去执行自己的校验
  3. 它的 value 不体现到最终 state 树的 value 里

其中，1 已经做到了，2 对应对应晨哥这个 PR 里的改动；至于 3，考虑到我们希望提供类型安全的 value，可预见的会比较麻烦，而且对表单页面的开发者意义不大，因为多给一个值几乎总是不会有问题的，他不去消费就好了，所以我理解 3 可以不去管

这样的话

就像从当前 state 树上被摘掉了一样

就不是特别严谨，严谨一点说是：被 disabled 的 state，就像从被当前 state 树上藏起来了一样；这个也很好理解，就像我们希望不展示一块界面的时候，也可能不是真的把它从 UI 树上摘掉，而是把它通过 css 藏起来

基于以上的前提，你上边提到的问题几乎都能得到一个自然的答案：

  • disabled 的时候 set reset （以及后面的 touched）这些接口该如何理解：就那么理解，你是在设置/操作一个被藏起来的状态，就像你操作一块被藏起来的界面一样
  • 比如一个 disabled 的 state 作为子 state 存在的时候，父 state 的 value 应该能拿到它的值吗：上面针对 value 说明过
  • disabled 的时候要不要不清 error：不用，没有必要，如果需要可以业务自己去做
  • 如果不清，那么转为 enabled 的那一刻，应该清了重新等 validate 或干脆不 active 吗：不用特别处理，保留原状态就好
  • 如果都不清，那么 disable when 变更的心智模型就跟 validators 变更有区别了，这确实无所谓吗：无所谓，本来这俩就不是一样的概念..

Copy link
Collaborator

Choose a reason for hiding this comment

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

就像从被当前 state 树上摘掉了一样

这你是从哪里引用到的…

Copy link
Collaborator

Choose a reason for hiding this comment

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

自己说的，自己引用..

}

const validation = this.validation

action('activate-and-sync-_value-when-validate', () => {
Expand All @@ -172,11 +187,7 @@ export default class FieldState<TValue> extends Disposable implements Composible
{ name: 'return-validate-when-not-validating' }
)

return (
this.hasError
? { hasError: true, error: this.error } as const
: { hasError: false, value: this.value } as const
)
return this.validateResult
}

/**
Expand Down
12 changes: 12 additions & 0 deletions src/formState.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,18 @@ describe('FormState (mode: object) validation', () => {
expect(state.ownError).toBeUndefined()
expect(state.error).toBeUndefined()

runInAction(() => options.disabled = false)

await delay()
expect(state.validating).toBe(false)
expect(state.validated).toBe(false)
expect(state.hasError).toBe(false)
expect(state.ownError).toBeUndefined()
expect(state.error).toBeUndefined()

runInAction(() => options.disabled = true)

await delay()
state.$.foo.onChange('')
await state.validate()
expect(state.hasOwnError).toBe(false)
Expand Down
21 changes: 16 additions & 5 deletions src/formState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ export default class FormState<TFields extends ValidatableFields, TValue = Value
return !!this.error
}

/**
* The most recent validation result.
* If state is disabled and later enabled, the result of the last validation is obtained.
* If you need to clear the validation result, please use the reset function.
*/
@computed private get validateResult(): ValidateResult<TValue> {
return this.hasError
? { hasError: true, error: this.error } as const
: { hasError: false, value: this.value } as const
}

/**
* If the validation has been done.
* It does not means validation passed.
Expand Down Expand Up @@ -219,6 +230,10 @@ export default class FormState<TFields extends ValidatableFields, TValue = Value
* Fire a validation behavior.
*/
async validate(): Promise<ValidateResult<TValue>> {
if (this.validationDisabled) {
return this.validateResult
}

action('activate-when-validate', () => {
this._activated = true
})()
Expand All @@ -234,11 +249,7 @@ export default class FormState<TFields extends ValidatableFields, TValue = Value
{ name: 'return-validate-when-not-validating' }
)

return (
this.hasError
? { hasError: true, error: this.error } as const
: { hasError: false, value: this.value } as const
)
return this.validateResult
}

/**
Expand Down
1 change: 0 additions & 1 deletion src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export interface Validatable<T, TValue = T> {
validated: boolean
validationDisabled: boolean
validate(): Promise<ValidateResult<TValue>>

// To see if there are requirements: enableAutoValidation, disableAutoValidation
// enableAutoValidation: () => void
// disableAutoValidation: () => void
Expand Down