-
Notifications
You must be signed in to change notification settings - Fork 9
[RMBWEB-2780] Support for structured validation results #87
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.
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
Changes from 4 commits
83cd99a
8d68591
f466115
db9f109
227f425
d84c44a
90e4499
78f35e5
b2f88ba
e5afe2a
3e45f64
511d66f
b8fb666
9065ddf
5d56725
2c76ca8
a8cb353
031f8a1
ba4b45e
dbba8f5
a5ec69d
82e505b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +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' | ||
|
|
||
| interface IV3StateFromV2<T extends v2.ComposibleValidatable<unknown, V>, V> extends v3.IState<V> { | ||
| /** The original ([email protected]) state */ | ||
|
|
@@ -27,6 +28,7 @@ class Upgrader<T extends v2.ComposibleValidatable<unknown, V>, V> extends BaseSt | |
| @computed get ownError() { | ||
| return getV3OwnError(this.stateV2) | ||
| } | ||
| @computed get rawError() { return this.ownError } | ||
| @computed get error() { return this.stateV2.error } | ||
| @computed get activated() { return this.stateV2._activated } | ||
| @computed get validateStatus() { | ||
|
|
@@ -47,7 +49,7 @@ class Upgrader<T extends v2.ComposibleValidatable<unknown, V>, V> extends BaseSt | |
| isV2FieldState(this.stateV2) | ||
| || isV2FormState(this.stateV2) | ||
| ) { | ||
| this.stateV2.validators(...validators) | ||
| this.stateV2.validators(...portV2Validators(...validators)) | ||
| return this | ||
| } | ||
| throwNotSupported() | ||
|
|
@@ -64,7 +66,23 @@ class Upgrader<T extends v2.ComposibleValidatable<unknown, V>, V> extends BaseSt | |
| } | ||
| } | ||
|
|
||
| /** Convets [email protected] state to [email protected] state */ | ||
| 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) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -373,4 +373,31 @@ describe('FieldState validation', () => { | |
| assertType<string>(res.value) | ||
| } | ||
| }) | ||
|
|
||
| describe('should work well with resolved error object', () => { | ||
| it('should work well with sync resolved', async () => { | ||
| const state = new FieldState('').withValidator( | ||
| _ => ({ message: 'error-object-msg'}) | ||
|
||
| ) | ||
|
|
||
| const res = await state.validate() | ||
| expect(state.hasError).toBe(true) | ||
| expect(state.error).toBe('error-object-msg') | ||
| expect(state.rawError).toEqual({ message: 'error-object-msg'}) | ||
| expect(res).toEqual({ hasError: true, error: 'error-object-msg', rawError: { message: 'error-object-msg' }}) | ||
| }) | ||
|
|
||
| it('should work well with async resolved', async () => { | ||
| const state = new FieldState('').withValidator( | ||
| _ => null, | ||
| _ => delayValue({ message: 'error-object-msg' }, 100) | ||
| ) | ||
|
|
||
| const res = await state.validate() | ||
| expect(state.hasError).toBe(true) | ||
| expect(state.error).toBe('error-object-msg') | ||
| expect(state.rawError).toEqual({ message: 'error-object-msg'}) | ||
| expect(res).toEqual({ hasError: true, error: 'error-object-msg', rawError: { message: 'error-object-msg' }}) | ||
| }) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||
| import { action, autorun, computed, makeObservable, observable, when } from 'mobx' | ||||||
| import { ValidationError, IState, Validation, ValidateResult, ValidateStatus, Validator } from './types' | ||||||
| import { ValidationRawError, ValidationError, IState, Validation, ValidateResult, ValidateStatus, Validator } from './types' | ||||||
| import Disposable from './disposable' | ||||||
| import { applyValidators, isValid, isPromiseLike } from './utils' | ||||||
| import { applyValidators, isPromiseLike, normalizeRawError, normalizeError } from './utils' | ||||||
|
|
||||||
| /** Extraction for some basic features of State */ | ||||||
| export abstract class BaseState extends Disposable implements Pick< | ||||||
|
|
@@ -56,10 +56,14 @@ export abstract class ValidatableState<V> extends BaseState implements IState<V> | |||||
| /** | ||||||
| * The original error info of validation. | ||||||
| */ | ||||||
| @observable protected _error: ValidationError | ||||||
| @observable protected _error: ValidationRawError | ||||||
|
|
||||||
| @computed get rawError() { | ||||||
| return this.disabled ? undefined : this._error | ||||||
|
||||||
| return this.disabled ? undefined : this._error | |
| return this.disabled ? undefined : this._error |
Outdated
There was a problem hiding this comment.
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) 这个逻辑都是成立的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不太明确 BaseState 的定位,我看 BaseState 上一些属性,跟 ValidatableState 关系也挺大的
There was a problem hiding this comment.
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 信息
There was a problem hiding this comment.
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 都成立的
There was a problem hiding this comment.
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 成立),那走这个逻辑会对实现更方便?
There was a problem hiding this comment.
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 成立),那走这个逻辑会对实现更方便?
我之前在这里就说了啊..,现在要把实现改回去吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我之前在这里就说了啊..
那边我没看懂你的意思..
现在要把实现改回去吗?
我倾向改回去,如果确实 hasError = !!error 这个逻辑还成立,而且基于这个逻辑确实更方便的话
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ba4b45e
done
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -389,6 +389,19 @@ describe('TransformedState (for FieldState) validation', () => { | |
| expect(state.ownError).toBe('non positive') | ||
| }) | ||
|
|
||
| it('should work well with resolved error object', async () => { | ||
| const state = createNumState(0).withValidator( | ||
| _ => ({ message: 'empty' }) | ||
| ) | ||
|
|
||
| state.validate() | ||
|
|
||
| await delay() | ||
|
||
| expect(state.hasError).toBe(true) | ||
| expect(state.error).toBe('empty') | ||
| expect(state.ownError).toBe('empty') | ||
| expect(state.rawError).toEqual({ message: 'empty' }) | ||
| }) | ||
| }) | ||
|
|
||
| interface Host { | ||
|
|
@@ -738,4 +751,18 @@ describe('TransformedState (for FormState) validation', () => { | |
| expect(state.hasOwnError).toBe(false) | ||
| expect(state.ownError).toBeUndefined() | ||
| }) | ||
|
|
||
| it('should work well with resolved error object', async () => { | ||
| const state = createHostState('127.0.0.1').withValidator( | ||
| _ => ({ message: 'mock msg'}) | ||
| ) | ||
|
|
||
| state.validate() | ||
|
|
||
| await delay() | ||
| expect(state.hasError).toBe(true) | ||
| expect(state.error).toBe('mock msg') | ||
| expect(state.ownError).toBe('mock msg') | ||
| expect(state.rawError).toEqual({ message: 'mock msg' }) | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在实现
DebouncedState的时候,也应该去通过 overriderawError来实现,而不是 overrideownError?如 #87 (comment) 这里所述,
ownError = normalizeError(rawError)这个逻辑对于DebouncedState也应该是成立的?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
没注意这里有个 override,我看怎么改下。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
031f8a1
done