Skip to content

fix(form-core): prevent removeValue from auto-touching shifted siblings #2133

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.

Sign up for GitHub

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
mixelburg wants to merge 2 commits into
base: main
Choose a base branch
from
+31 −2
Open

fix(form-core): prevent removeValue from auto-touching shifted siblings #2133

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd491ba
fix(form-core): prevent removeValue from auto-touching shifted siblings
mixelburg Apr 18, 2026
a1294aa
fix(form-core): properly scope avoidTouch to removeFieldValue only
mixelburg Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions packages/form-core/src/FormApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1582,6 +1582,7 @@ export class FormApi<
field: TField,
index: number,
cause: ValidationCause,
options?: { avoidTouch?: boolean },
) => {
const currentValue = this.getFieldValue(field)

Expand All @@ -1605,7 +1606,32 @@ export class FormApi<
batch(() => {
fieldsToValidate.forEach((nestedField) => {
fieldValidationPromises.push(
Promise.resolve().then(() => this.validateField(nestedField, cause)),
Promise.resolve().then(async () => {
// If avoidTouch is set and the field instance already exists,
// run validation without permanently marking the field as touched
if (options?.avoidTouch) {
const fieldInstance = this.fieldInfo[nestedField]?.instance
if (fieldInstance) {
const wasTouched = fieldInstance.state.meta.isTouched
if (!wasTouched) {
// Temporarily touch the field so validation runs, then restore
fieldInstance.setMeta((prev) => ({
...prev,
isTouched: true,
}))
}
const result = await fieldInstance.validate(cause)
if (!wasTouched) {
fieldInstance.setMeta((prev) => ({
...prev,
isTouched: false,
}))
}
return result
}
}
return this.validateField(nestedField, cause)
}),
Comment on lines +1609 to +1634
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard restoration with try/finally, and note the transient-state exposure.

Two concerns with the temporary-touch approach:

  1. No error safety. If fieldInstance.validate(cause) throws (sync throw before returning a Promise, or a rejected Promise returned synchronously before the await), the second setMeta never runs and the field is left permanently isTouched: true — reintroducing the exact bug this PR fixes. Wrap in try/finally.
  2. Observable transient state. Between the two setMeta calls, the derived stores (fieldMetaDerived, store.isTouched, devtools broadcast via throttleFormState) momentarily reflect isTouched: true for every untouched sibling. With N shifted siblings all toggling in parallel, subscribers/listeners can observe a spurious "all touched" window. Also, if the user genuinely touches the field during the await, the restoration will clobber their touch back to false.

The try/finally is a strict correctness fix and should be applied. The transient-state issue is the main reason the originally suggested refactor (an internal { touch: false } path on validateField that skips the isTouched check entirely) is cleaner — worth reconsidering as a follow-up.

🛡️ Proposed fix: ensure restoration on error paths 
             if (options?.avoidTouch) {
               const fieldInstance = this.fieldInfo[nestedField]?.instance
               if (fieldInstance) {
                 const wasTouched = fieldInstance.state.meta.isTouched
                 if (!wasTouched) {
                   // Temporarily touch the field so validation runs, then restore
                   fieldInstance.setMeta((prev) => ({
                     ...prev,
                     isTouched: true,
                   }))
                 }
-                const result = await fieldInstance.validate(cause)
-                if (!wasTouched) {
-                  fieldInstance.setMeta((prev) => ({
-                    ...prev,
-                    isTouched: false,
-                  }))
-                }
-                return result
+                try {
+                  return await fieldInstance.validate(cause)
+                } finally {
+                  if (!wasTouched) {
+                    fieldInstance.setMeta((prev) => ({
+                      ...prev,
+                      isTouched: false,
+                    }))
+                  }
+                }
               }
             }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/form-core/src/FormApi.ts` around lines 1609 - 1634, The
temporary-touch logic inside the Promise.resolve block when options?.avoidTouch
is true should ensure the original isTouched is always restored even if
fieldInstance.validate(cause) throws or rejects: wrap the validate call and the
second setMeta restoration in a try/finally so that after temporarily setting
fieldInstance.setMeta(... isTouched: true) you always run the restore
setMeta(...) in the finally block; keep the early-return of the validate result,
and otherwise fall back to this.validateField(nestedField, cause). Reference the
Promise.resolve(...) block, options?.avoidTouch, fieldInstance,
fieldInstance.validate(cause), and fieldInstance.setMeta(...) when making the
change.

)
})
})
Expand Down Expand Up @@ -2469,7 +2495,10 @@ export class FormApi<
if (!dontValidate) {
// Validate the whole array + all fields that have shifted
await this.validateField(field, 'change')
await this.validateArrayFieldsStartingFrom(field, index, 'change')
// avoidTouch: true prevents auto-touching siblings that shifted after removal
await this.validateArrayFieldsStartingFrom(field, index, 'change', {
avoidTouch: true,
})
}
}

Expand Down