form icon indicating copy to clipboard operation
form copied to clipboard

feat (form-core): Merging Form-level server errors with Field-level errors

Open theVedanta opened this issue 8 months ago • 3 comments

Instead of the previous serverValidate function just returning a string, we now return an object:

const serverValidate = createServerValidate({
  ...formOpts,
  onServerValidate: ({ value }) => {
    if (value.age < 12)
      return { age: 'Server validation: You must be at least 12 to sign up' }
  },
})

This then maps to the field-level errors with the setMeta options.

The only reason this is called a preliminary approach is because we are setting the onServer key of the errorMap differently.

Previously, the error map for the form looked like so:

{
  onServer: "You need to be 12 years or older to sign up"
}

Now however, the onServer map is slightly different:

{
  onServer: { age: "You need to be 12 years or older to sign up" }
}

This causes some degree of inconsistency, which is why I would love some feedback on whether we can use some other property (or maybe create our own!) to manage the errors between fields and forms. This would not be a comprehensive property but just a map to sync them back and forth.

NextJS support has been pushed. Remix and Start coming soon. Support for Array field types will also be done shortly.

Would love your feedback and comments on this approach before I proceed!

Solves the following issue: #1260

theVedanta avatar Apr 20 '25 19:04 theVedanta

View your CI Pipeline Execution ↗ for commit d06c1ff272160e61ad5fc74a6bae370394d32f6a

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

☁️ Nx Cloud last updated this comment at 2025-10-31 21:16:04 UTC

nx-cloud[bot] avatar Apr 20 '25 19:04 nx-cloud[bot]

More templates

@tanstack/angular-form

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

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

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

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

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

npm i https://pkg.pr.new/@tanstack/react-form-devtools@1432
@tanstack/solid-form

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

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

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

commit: d06c1ff

pkg-pr-new[bot] avatar Apr 20 '25 19:04 pkg-pr-new[bot]

Codecov Report

:x: Patch coverage is 7.14286% with 39 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 89.10%. Comparing base (6892ed0) to head (d06c1ff). :warning: Report is 70 commits behind head on main.

Files with missing lines Patch % Lines
packages/form-core/src/FormApi.ts 18.75% 10 Missing and 3 partials :warning:
...ages/react-form/src/nextjs/createServerValidate.ts 0.00% 10 Missing and 3 partials :warning:
...kages/react-form/src/remix/createServerValidate.ts 0.00% 10 Missing and 3 partials :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1432      +/-   ##
==========================================
- Coverage   90.35%   89.10%   -1.25%     
==========================================
  Files          38       38              
  Lines        1752     1836      +84     
  Branches      444      471      +27     
==========================================
+ Hits         1583     1636      +53     
- Misses        149      173      +24     
- Partials       20       27       +7     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Apr 20 '25 19:04 codecov[bot]

Hello, do you need a hand to move this branch forward? I would love to see it happen :)

vincehi avatar Sep 16 '25 09:09 vincehi

Hello, do you need a hand to move this branch forward? I would love to see it happen :)

Yeah sure dude, go for it!

theVedanta avatar Sep 17 '25 23:09 theVedanta

Not sure if this feature is still requested or whether it has been fixed. Don't wanna tag maintainers but this has been dormant for a while, I'd like some insight on how to proceed! @LeCarbonator @harry-whorlow

theVedanta avatar Oct 19 '25 18:10 theVedanta

The merge conflicts appear to be in some of the important diffs, so I'll leave that up to you. I'll see if convenient unit tests could be made for these as we don't want to regress on this.

LeCarbonator avatar Oct 24 '25 09:10 LeCarbonator

Sorry for the long delay! I'm not too confident in my SSR knowledge, so I'll make sure to give this a try myself. I'll also go through some issues and the reproductions to confirm things.

Hopefully, this initial review should help with some refactoring. It's looking promising!

Since this has been stale for way too long, this will be my focus for now. Feel free to tag me if there's more info you need or a second review. I can also help out with the PR if you'd like.

Your help with the PR would be awesome! I'm pretty new to learning about the tanstack ecosystem. I'll try to make another commit with the fixes.

theVedanta avatar Oct 28 '25 03:10 theVedanta

⚠️ No Changeset found

Latest commit: d06c1ff272160e61ad5fc74a6bae370394d32f6a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Oct 28 '25 03:10 changeset-bot[bot]

@LeCarbonator just following up on this :)

theVedanta avatar Nov 19 '25 19:11 theVedanta

@theVedanta I'm super sorry about this, but I've realized that our SSR story is DEEPLY broken (my fault, not related to this specifically).

As such, I'm prototyping new ideas here:

https://github.com/TanStack/form/pull/1890

While I don't want the general public to worry about any potential breaking changes yet (we'll have extensive migration guides and such), I'd love you thoughts on the approach I took there to fix things.

LMK!

Thanks a billion for your amazing help. Closing this PR to move discussions there

crutchcorn avatar Dec 01 '25 09:12 crutchcorn

@theVedanta I'm super sorry about this, but I've realized that our SSR story is DEEPLY broken (my fault, not related to this specifically).

As such, I'm prototyping new ideas here:

#1890

While I don't want the general public to worry about any potential breaking changes yet (we'll have extensive migration guides and such), I'd love you thoughts on the approach I took there to fix things.

LMK!

Thanks a billion for your amazing help. Closing this PR to move discussions there

haha no worries man. and thanks so much for closing this. I ended up discussing this PR with Luca but totally forgot to close it! I would love to check out the awesome work you're doing to improve the SSR :)

theVedanta avatar Dec 01 '25 18:12 theVedanta