form icon indicating copy to clipboard operation
form copied to clipboard

Deduplicate field and form `errors` property

Open oscartbeaumont opened this issue 1 year ago • 3 comments

The current behavior of form.state.errors and field.getMeta().errors is to collect up all errors within the errorMap. The implementation does not take care of removing duplicates and Form's unit tests assert this behaviour as expected.

I think this behaviour should potentially be reconsidered.

Personally I think it's very common as an application developer to apply the same validator to multiple events (Eg, onMount & onChange) and if that is done the .errors array will likely end up with duplicates.

If the application developer is to render a list of all errors for the current field/form using .errors, they will end up showing the user duplicates. I don't think a user should ever be shown duplicates because it is only one error for them to fix. Right now the application developer would be responsible for implementing deduplication logic and I feel like .errors should just do this by default.

If a developer really want the previous behavior they can easily do Object.values(.errorMap) but I feel like keeping duplicates is something you should need to opt-in to, as opposed to opt-out like it is right now.

Happy to close this PR if the discussion around changing this behavior decides it's not a great idea.

oscartbeaumont avatar Aug 17 '24 04:08 oscartbeaumont

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b27ccce7bd8340a9a986abe7fe473149c0db767b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

nx-cloud[bot] avatar Aug 17 '24 04:08 nx-cloud[bot]

commit: b27ccce

@tanstack/angular-form

pnpm add https://pkg.pr.new/@tanstack/angular-form@908
@tanstack/form-core

pnpm add https://pkg.pr.new/@tanstack/form-core@908
@tanstack/lit-form

pnpm add https://pkg.pr.new/@tanstack/lit-form@908
@tanstack/react-form

pnpm add https://pkg.pr.new/@tanstack/react-form@908
@tanstack/solid-form

pnpm add https://pkg.pr.new/@tanstack/solid-form@908
@tanstack/valibot-form-adapter

pnpm add https://pkg.pr.new/@tanstack/valibot-form-adapter@908
@tanstack/vue-form

pnpm add https://pkg.pr.new/@tanstack/vue-form@908
@tanstack/yup-form-adapter

pnpm add https://pkg.pr.new/@tanstack/yup-form-adapter@908
@tanstack/zod-form-adapter

pnpm add https://pkg.pr.new/@tanstack/zod-form-adapter@908

Open in Stackblitz

More templates

pkg-pr-new[bot] avatar Aug 17 '24 04:08 pkg-pr-new[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.56%. Comparing base (5473bb8) to head (b27ccce). Report is 233 commits behind head on main.

:x: Your project status has failed because the head coverage (88.56%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #908      +/-   ##
==========================================
- Coverage   91.55%   88.56%   -2.99%     
==========================================
  Files          21       26       +5     
  Lines         900      936      +36     
  Branches      206      209       +3     
==========================================
+ Hits          824      829       +5     
- Misses         71      101      +30     
- Partials        5        6       +1     

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

codecov[bot] avatar Aug 17 '24 04:08 codecov[bot]

This should be much less of a problem now with the return anything. I'm also philosophically opposed to filtering out the errors array any more than we currently are 😅

crutchcorn avatar Feb 21 '25 14:02 crutchcorn