fix(react-form): removing array items (#1171)

[niba]

It is impossible to remove an item from an array while properly using React keys.

Here is an example taken from the issue that demonstrates the problem. Try to remove the middle item.

This issue occurs because we use the ⁠index inside the field name. When you remove any item other than the last one, the array is re-iterated, and in the next render, the field names change.
In the current code, you update the name as the last action and use the old name in earlier actions.

Let's assume you have an array: ⁠[{ id: 1, name: "one"}, {id: 2, name: "two"}, {id: 3, name: "three"}], and you remove the middle item. In the next render loop, you want to render ⁠[{ id: 1, name: "one"}, {id: 3, name: "three"}]. However, for the item with ⁠id = 3, the store tries to access the value using the name ⁠"items[2].value" instead of ⁠"items[1].value", which causes unexpected effects (see the example).

This bug doesn't occur when you use the index as a key (which is not correct). In this scenario, React behaves a little differently and doesn't cause a re-render.

I'm not familiar with the entire codebase, so I don't know if this change could have any side effects.

[crutchcorn]

Can we add tests to verify that we don't regress problems?

[niba]

Sure. Sorry, I got so excited that I managed to fix it after a long debugging session that I forgot about the tests. 😸

[niba]

@crutchcorn I added a test and found another problem that I resolved.

After my first change, the array fields started rendering correctly, but the displayed values were wrong.

The reason was that defaultValue was not updated after removing any item in the array. In the next render, FieldApi calls setValue with a defaultValue that is not valid because it belongs to the removed field.

The flow:

defaultValues: ["a", "b", "c"]

render: 
"a",
"b",
"c",

remove "b"

render:
"a"
"b" WRONG_VALUE ->  this.state.value is undefined, so it takes value from ⁠defaultValues where ⁠defaultValue[1] === "b". This one wins because the updater is more important than store calculation

To fix this I added new code that modifies how ⁠setValue is called when a name change is detected.

[nx-cloud]

View your CI Pipeline Execution ↗ for commit 39e82b3.

Command	Status	Duration	Result
nx affected --targets=test:sherif,test:knip,tes...	✅ Succeeded	2m 11s	View ↗

---

☁️ Nx Cloud last updated this comment at 2025-03-28 10:18:45 UTC

[pkg-pr-new]

Open in Stackblitz

More templates

• @tanstack/form-example-angular-array
• @tanstack/form-example-angular-simple
• @tanstack/form-example-lit-ui-libraries
• @tanstack/form-example-lit-simple
• @tanstack/form-example-react-array
• @tanstack/form-example-react-compiler
• @tanstack/field-errors-from-form-validators
• @tanstack/form-example-react-large-form
• @tanstack/form-example-react-next-server-actions
• @tanstack/form-example-react-query-integration
• @tanstack/form-example-remix
• @tanstack/form-example-react-simple
• @tanstack/form-example-react-standard-schema
• @tanstack/form-example-react-tanstack-start
• @tanstack/form-example-react-ui-libraries
• @tanstack/form-example-solid-array
• @tanstack/form-example-solid-simple
• @tanstack/form-example-vue-array
• @tanstack/form-example-vue-simple

@tanstack/angular-form

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

@tanstack/form-core

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

@tanstack/lit-form

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

@tanstack/react-form

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

@tanstack/solid-form

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

@tanstack/vue-form

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

commit: 39e82b3

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.88%. Comparing base (516f587) to head (39e82b3).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/form-core/src/FieldApi.ts	75.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1319      +/-   ##
==========================================
+ Coverage   88.86%   88.88%   +0.01%     
==========================================
  Files          28       28              
  Lines        1275     1277       +2     
  Branches      332      335       +3     
==========================================
+ Hits         1133     1135       +2     
+ Misses        127      126       -1     
- Partials       15       16       +1     

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

[Balastrong]

Can you please also add some tests to validate the new default logic?

At first the fix seems to be working, well done :)

[niba]

The line not covered by tests is an old logic line. I needed to add a new 'if' branch, so I refactored the code slightly to make it easier to understand.

The default value logic remains the same and should be covered by the tests introduced here: GitHub Pull Request #445.

I only added an exception to not use the default value when the name has changed. This is covered by the array tests, where I check if the values are correct after removing an element.

If you want, I can add some tests, but they would duplicate existing ones (setting the default value on mount).

Thanks!