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.

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

[WIP] [FEAT] Allow returning anything in a validator, not just a string #1104

Draft
wants to merge 46 commits into
base: main
Choose a base branch
from

Conversation

crutchcorn
Copy link
Member

@crutchcorn crutchcorn commented Jan 9, 2025

This PR allows you to return non-string values from validators:

<form.Field name="password" validators={{onChange: () => ({hasUppercase: false, hasLowercase: true}) }} />

In addition, it also enforces type safety on the returned value on both errorMap and the errors array:

const form = new FormApi({
  defaultValues: {
    name: 'test',
  },
} as const)

const field = new FieldApi({
  form,
  name: 'name',
  validators: {
    onChange: () => {
      return 123 as const
    },
  },
})

assertType<123 | undefined>(field.state.meta.errorMap.onChange)

assertType<Array<123 | undefined>>(field.state.meta.errors)

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

Copy link

nx-cloud bot commented Jan 9, 2025

View your CI Pipeline Execution ↗ for commit 0d40105.

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 1m 45s View ↗
nx run-many --target=build --exclude=examples/** ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-15 03:07:58 UTC

@crutchcorn crutchcorn changed the title [FIX] Allow returning anything in a validator, not just a string [WIP] [FIX] Allow returning anything in a validator, not just a string Jan 9, 2025
@crutchcorn crutchcorn marked this pull request as draft January 9, 2025 08:56
Copy link

pkg-pr-new bot commented Jan 9, 2025

Open in Stackblitz

More templates

@tanstack/angular-form

npm i https://pkg.pr.new/@tanstack/angular-form@1104

@tanstack/form-core

npm i https://pkg.pr.new/@tanstack/form-core@1104

@tanstack/react-form

npm i https://pkg.pr.new/@tanstack/react-form@1104

@tanstack/lit-form

npm i https://pkg.pr.new/@tanstack/lit-form@1104

@tanstack/solid-form

npm i https://pkg.pr.new/@tanstack/solid-form@1104

@tanstack/valibot-form-adapter

npm i https://pkg.pr.new/@tanstack/valibot-form-adapter@1104

@tanstack/vue-form

npm i https://pkg.pr.new/@tanstack/vue-form@1104

@tanstack/yup-form-adapter

npm i https://pkg.pr.new/@tanstack/yup-form-adapter@1104

@tanstack/zod-form-adapter

npm i https://pkg.pr.new/@tanstack/zod-form-adapter@1104

commit: 0d40105

@crutchcorn crutchcorn changed the title [WIP] [FIX] Allow returning anything in a validator, not just a string [WIP] [FEAT] Allow returning anything in a validator, not just a string Jan 10, 2025
@crutchcorn crutchcorn force-pushed the return-anything-not-just-string branch from 20e5b68 to c9ea6cf Compare January 11, 2025 17:59
Copy link

codecov bot commented Jan 12, 2025

Codecov Report

Attention: Patch coverage is 67.56757% with 24 lines in your changes missing coverage. Please review.

Project coverage is 88.70%. Comparing base (08d610f) to head (0d40105).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ages/react-form/src/start/createServerValidate.tsx 0.00% 5 Missing and 1 partial ⚠️
...ages/react-form/src/nextjs/createServerValidate.ts 0.00% 4 Missing and 1 partial ⚠️
...kages/react-form/src/remix/createServerValidate.ts 0.00% 3 Missing and 1 partial ⚠️
packages/react-form/src/start/getFormData.tsx 0.00% 3 Missing ⚠️
packages/form-core/src/FormApi.ts 91.30% 2 Missing ⚠️
packages/react-form/src/nextjs/error.ts 0.00% 1 Missing ⚠️
packages/react-form/src/remix/error.ts 0.00% 1 Missing ⚠️
packages/react-form/src/start/error.ts 0.00% 1 Missing ⚠️
packages/react-form/src/useTransform.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1104      +/-   ##
==========================================
+ Coverage   86.70%   88.70%   +2.00%     
==========================================
  Files          29       29              
  Lines        1181     1160      -21     
  Branches      295      281      -14     
==========================================
+ Hits         1024     1029       +5     
+ Misses        144      119      -25     
+ Partials       13       12       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@fulopkovacs fulopkovacs left a comment

Choose a reason for hiding this comment

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

Ok, I'm done with reviewing core. I didn't really find any issues so far, left only 1 comment in the tests.

I'll continue the review next week with the react-form package!

packages/form-core/tests/FormApi.spec.ts Show resolved Hide resolved
Copy link
Member

@Balastrong Balastrong left a comment

Choose a reason for hiding this comment

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

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:

  1. We must find a way to keep the types more maintainable, the idea kevin suggested about type bags might be worth exploring
  2. Schema validators inferring undefined is probably a blocker, but I feel bad to ask for even more types refactor in this PR
  3. The AnyFieldApi type 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.
  4. 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.
  5. 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> = {
Copy link
Member

Choose a reason for hiding this comment

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

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 = (
Copy link
Member

Choose a reason for hiding this comment

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

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>
Copy link
Member

Choose a reason for hiding this comment

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

This type should probably be exported from form-core

# Conflicts:
#	docs/framework/angular/reference/classes/tanstackfield.md
#	docs/framework/angular/reference/functions/injectform.md
#	docs/framework/lit/reference/classes/tanstackformcontroller.md
#	docs/framework/react/reference/functions/field.md
#	docs/framework/react/reference/functions/usefield.md
#	docs/framework/react/reference/functions/useform.md
#	docs/framework/react/reference/functions/usestore.md
#	docs/framework/react/reference/functions/usetransform.md
#	docs/framework/react/reference/interfaces/reactformapi.md
#	docs/framework/react/reference/type-aliases/fieldcomponent.md
#	docs/framework/react/reference/type-aliases/reactformextendedapi.md
#	docs/framework/react/reference/type-aliases/usefield.md
#	docs/framework/solid/reference/functions/createfield.md
#	docs/framework/solid/reference/functions/createform.md
#	docs/framework/solid/reference/functions/field.md
#	docs/framework/solid/reference/functions/usestore.md
#	docs/framework/solid/reference/interfaces/solidformapi.md
#	docs/framework/solid/reference/type-aliases/createfield.md
#	docs/framework/solid/reference/type-aliases/fieldcomponent.md
#	docs/framework/vue/reference/functions/usefield.md
#	docs/framework/vue/reference/functions/useform.md
#	docs/framework/vue/reference/functions/usestore.md
#	docs/framework/vue/reference/interfaces/vuefieldapi.md
#	docs/framework/vue/reference/interfaces/vueformapi.md
#	docs/framework/vue/reference/type-aliases/fieldcomponent.md
#	docs/framework/vue/reference/type-aliases/usefield.md
#	docs/framework/vue/reference/variables/field.md
#	docs/reference/classes/fieldapi.md
#	docs/reference/classes/formapi.md
#	docs/reference/functions/mergeform.md
#	docs/reference/interfaces/fieldapioptions.md
#	docs/reference/interfaces/fieldlisteners.md
#	docs/reference/interfaces/fieldoptions.md
#	docs/reference/interfaces/fieldvalidators.md
#	docs/reference/interfaces/formoptions.md
#	docs/reference/interfaces/formvalidators.md
#	docs/reference/type-aliases/baseformstate.md
#	docs/reference/type-aliases/derivedformstate.md
#	docs/reference/type-aliases/fieldinfo.md
#	docs/reference/type-aliases/fieldmeta.md
#	docs/reference/type-aliases/fieldmetabase.md
#	docs/reference/type-aliases/fieldmetaderived.md
#	docs/reference/type-aliases/fieldserrormapfromvalidator.md
#	docs/reference/type-aliases/fieldstate.md
#	docs/reference/type-aliases/formstate.md
#	docs/reference/type-aliases/formvalidatefn.md
#	docs/reference/type-aliases/formvalidator.md
#	docs/reference/type-aliases/validationerror.md
#	docs/reference/type-aliases/validationmeta.md
#	examples/react/compiler/package.json
#	examples/react/next-server-actions/package.json
#	examples/react/tanstack-start/package.json
#	examples/solid/array/package.json
#	examples/solid/simple/package.json
#	examples/solid/valibot/package.json
#	examples/solid/yup/package.json
#	examples/solid/zod/package.json
#	examples/vue/array/package.json
#	examples/vue/simple/package.json
#	examples/vue/valibot/package.json
#	examples/vue/yup/package.json
#	examples/vue/zod/package.json
#	package.json
#	packages/react-form/package.json
#	packages/vue-form/package.json
#	pnpm-lock.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update TanStack Start adapter
3 participants