[WIP] [FEAT] Allow returning anything in a validator, not just a string #1104
Conversation
|
View your CI Pipeline Execution ↗ for commit a5d41db.
☁️ Nx Cloud last updated this comment at |
crutchcorn
changed the title
crutchcorn
changed the title
crutchcorn
force-pushed
the
return-anything-not-just-string
branch
from
20e5b68 to
c9ea6cf
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1104 +/- ##
==========================================
+ Coverage 86.61% 88.62% +2.01%
==========================================
Files 29 29
Lines 1173 1152 -21
Branches 288 276 -12
==========================================
+ Hits 1016 1021 +5
+ Misses 144 119 -25
+ Partials 13 12 -1 ☔ View full report in Codecov by Sentry. |
| @@ -1573,7 +1573,7 @@ describe('form api', () => { | |||
| form.mount() | |||
| form.setErrorMap({ | |||
| onChange: "name can't be Josh", | |||
| }) | |||
| } as never) | |||
There was a problem hiding this comment.
I'm still getting familiar with the PR, so maybe I don't have the full picture, but I think it'd be nice to get rid of the as never type assertions.
This seems to work:
it('should replace errorMap value if it exists in the FormApi object', () => {
const form = new FormApi({
validators: {
onChange: () => {
return "name can't be Josh"
},
},
})
form.mount()
form.setErrorMap({
onChange: "name can't be Josh",
})
expect(form.state.errorMap.onChange).toEqual("name can't be Josh")
form.setErrorMap({
onChange: 'other validation error',
})
expect(form.state.errorMap.onChange).toEqual('other validation error')
})
There was a problem hiding this comment.
That's a massive typescript rework :D
I love the idea that everyone can decide what to return and gets magically inferred, looks like it's working great!
A few more detailed considerations:
- We must find a way to keep the types more maintainable, the idea kevin suggested about type bags might be worth exploring
- Schema validators inferring undefined is probably a blocker, but I feel bad to ask for even more types refactor in this PR
- The
AnyFieldApitype is convenient but in userland means errors are all any in custom components like our FieldInfo we use in examples. We should expose some utility types or better guidance to cover that scenario. - I'd probably write a few more tests to ensure the errors array works fine even with mixed error types (both the js content is correct and the ts types are inferred right). Anyway I did a quick check manually and seems ok.
- We should definitely add a fully working example showcasing this powerful api and explain it more in detail in the docs!
To recap, I feel 2 might be a blocker, 3 might not and everything else can be tackled in a new PR.
| form?: ValidationError | ||
| fields: Partial<Record<DeepKeys<TFormData>, ValidationError>> | ||
| } | ||
| export type SpecialFormValidationError<TFormData> = { |
There was a problem hiding this comment.
nitpick: I feel like we can find a slightly less generic name than "special", maybe composite? global? ok they're still super generic anyway :D
| @@ -324,3 +324,9 @@ export function getSyncValidatorArray<T>( | |||
| return [changeValidator, serverValidator] as never | |||
| } | |||
| } | |||
|
|
|||
| export const isFormValidationError = ( | |||
There was a problem hiding this comment.
The function name should match the type's name since we're changing it
| > & { | ||
| mode?: 'value' | 'array' | ||
| } | ||
|
|
||
| export type AnyFormState = FormState<any, any, any, any, any, any, any, any> |
There was a problem hiding this comment.
This type should probably be exported from form-core
This PR allows you to return non-string values from validators:
In addition, it also enforces type safety on the returned value on both
errorMapand theerrorsarray:Start Adapter Fixes
Along the way, I wanted to fix the Start adapter, so we're doing so in this PR. Fixes: #1023
Vue typings
This PR also fixes the issues with Vue SFC format and marks Vue 3.4 as the new minimum supported version
TODO
errorarray handling