Skip to content

fix(core): fix deleteField method #1706

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kusiewicz
Copy link
Contributor

@kusiewicz kusiewicz commented Aug 19, 2025

During implementing this and testing deleteField I found out that this method does not work.

When a form field has onChange validation and the deleteField method is called on it, an error appears.

FormApi.ts:1586 Uncaught TypeError: Cannot read properties of undefined (reading 'errorMap')
    at FormApi.ts:1586:27
    at functionalUpdate (utils.ts:26:8)
    at FormApi.ts:2058:20

error: https://bolt.new/~/vitejs-vite-ceu2fl73

I think flow look like this:

  1. Remove button click => form.deleteField('firstName').
  2. Delete function changes values so onChange validator runs.
  3. This validator runs on the form level and still tries to set errors.fields.email = 'error message' - so validateSync tries to update metadata of deleted field.
  4. setFieldMeta calls functionalUpdate with undefined (since field does not exists in fieldMetaBase).
  5. Updater tries to do ...prev where prev now is undefined -> error.

So validateSync iterates through field list that already semes to be outdated. There is a check if (!fieldMeta) in this function but I guess we have to add as well if (!fieldMetaBase).

@kusiewicz kusiewicz marked this pull request as draft August 19, 2025 18:17
Copy link

nx-cloud bot commented Aug 19, 2025

View your CI Pipeline Execution ↗ for commit dfd06c5

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

☁️ Nx Cloud last updated this comment at 2025-08-19 18:20:30 UTC

Copy link

pkg-pr-new bot commented Aug 19, 2025

More templates

@tanstack/angular-form

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

@tanstack/form-core

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

@tanstack/lit-form

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

@tanstack/react-form

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

@tanstack/solid-form

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

@tanstack/svelte-form

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

@tanstack/vue-form

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

commit: dfd06c5

Copy link

codecov bot commented Aug 19, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.48%. Comparing base (d3a1b59) to head (dfd06c5).

Files with missing lines Patch % Lines
packages/form-core/src/FormApi.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1706      +/-   ##
==========================================
- Coverage   90.52%   90.48%   -0.04%     
==========================================
  Files          37       37              
  Lines        1688     1692       +4     
  Branches      421      423       +2     
==========================================
+ Hits         1528     1531       +3     
- Misses        143      144       +1     
  Partials       17       17              

☔ 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.

@kusiewicz kusiewicz marked this pull request as ready for review August 21, 2025 08:18
@kusiewicz
Copy link
Contributor Author

Please, verify the idea. If I'm thinking in a proper direction I will proceed and add unit tests.

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.

1 participant