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

fix(form-core): fix broken sync/async validation state not shifting with array actions (remove,swap,shift,move...) #1324

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

Conversation

juanvilladev
Copy link
Contributor

@juanvilladev juanvilladev commented Mar 23, 2025

Previously we were using a cumulative field errors map maintained at the form api level.
This PR removes that data structure in favor of a errorSourceMap data structure that lives within the FieldMeta for each field. This allows shifting/removing/deleting fields to automatically keep the errorSourceMap in sync.

This will fix #1323 and prevent other bugs from being raised around sync validation not working after other array actions like removing, shifting, swapping, moving, etc...

Copy link

nx-cloud bot commented Mar 23, 2025

View your CI Pipeline Execution ↗ for commit a3bc878.

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

☁️ Nx Cloud last updated this comment at 2025-03-28 05:15:44 UTC

Copy link

pkg-pr-new bot commented Mar 23, 2025

Copy link

codecov bot commented Mar 23, 2025

Codecov Report

Attention: Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.88%. Comparing base (516f587) to head (14b391a).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/form-core/src/FieldApi.ts 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1324      +/-   ##
==========================================
+ Coverage   88.86%   88.88%   +0.01%     
==========================================
  Files          28       28              
  Lines        1275     1286      +11     
  Branches      332      340       +8     
==========================================
+ Hits         1133     1143      +10     
- Misses        127      128       +1     
  Partials       15       15              

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

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

@juanvilladev juanvilladev marked this pull request as draft March 23, 2025 19:02
@juanvilladev juanvilladev force-pushed the fix/1323_clean_up_cumulative_fields_map_on_delete_and_reset branch from fd9f83a to f19d461 Compare March 23, 2025 20:58
@juanvilladev juanvilladev changed the title fix(form-core): handle syncing cumulative errors map on field deletion and reset fix(form-core): fix broken sync validation state not shifting with array actions (remove,swap,shift,move...) Mar 23, 2025
@juanvilladev juanvilladev marked this pull request as ready for review March 23, 2025 21:02
@juanvilladev juanvilladev marked this pull request as draft March 23, 2025 21:23
@juanvilladev juanvilladev marked this pull request as ready for review March 23, 2025 21:56
@juanvilladev juanvilladev changed the title fix(form-core): fix broken sync validation state not shifting with array actions (remove,swap,shift,move...) fix(form-core): fix broken sync/async validation state not shifting with array actions (remove,swap,shift,move...) Mar 23, 2025
@juanvilladev
Copy link
Contributor Author

Async validation & sync validation now is fully synced and thoroughly tested with the new errorSourceMap. This reduces the complexity of using the cumulative errors map significantly and fixes all the issues with validations running on the wrong fields when removing/inserting/moving/shifting array fields.

Please review when you have time!

Thanks!

@juanvilladev
Copy link
Contributor Author

juanvilladev commented Mar 28, 2025

Whoops messed up the rebase, will fix tomorrow

This commit updates async validators to ensure source map is synced with full coverage.

Fixes "Cannot read properties of undefined" when removing array field TanStack#1323
@juanvilladev juanvilladev force-pushed the fix/1323_clean_up_cumulative_fields_map_on_delete_and_reset branch from bb62f3c to e79b51b Compare March 28, 2025 05:10
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.

"Cannot read properties of undefined" when removing array field
1 participant