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

Deduplicate field and form errors property #908

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

oscartbeaumont
Copy link
Contributor

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.

Copy link

nx-cloud bot commented Aug 17, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b27ccce. 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.

Copy link

pkg-pr-new bot commented Aug 17, 2024

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

Copy link

codecov bot commented Aug 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

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     

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

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.

Nice catch, having duplicate errors isn't great from an user's perspective.

Could you please tweak the unit tests on this? At least to have one that keeps passing with different errors from different validators, and one that shows how duplicate errors are shown just once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants