Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
86 changes: 86 additions & 0 deletions packages/form-core/tests/FormApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,92 @@ describe('form api', () => {
expect(form.getFieldValue('names')).toStrictEqual(['one', 'two', 'three'])
})

it("should clean onMount errors when replacing an array field's value", async () => {
const form = new FormApi({
defaultValues: {
people: [
{
firstName: '',
lastName: '',
},
],
},
validators: {
onMount: ({ value }) => {
let fieldErrors: Record<string, string> = {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use const for the validator error maps.

fieldErrors is mutated but never reassigned, and ESLint is failing on Lines 381 and 404.

Proposed fix
-          let fieldErrors: Record<string, string> = {}
+          const fieldErrors: Record<string, string> = {}

-          let fieldErrors: Record<string, string> = {}
+          const fieldErrors: Record<string, string> = {}

Also applies to: 404-404

🧰 Tools
🪛 ESLint

[error] 381-381: 'fieldErrors' is never reassigned. Use 'const' instead.

(prefer-const)

🪛 GitHub Actions: PR

[error] 381-381: ESLint (prefer-const): 'fieldErrors' is never reassigned. Use 'const' instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/form-core/tests/FormApi.spec.ts` at line 381, The test declares the
validator error maps with "let fieldErrors" but they are only mutated
(properties updated) and never reassigned; change the declaration of fieldErrors
to "const fieldErrors" in the test(s) (references to fieldErrors around the
assertions at the locations shown) so ESLint no longer flags reassignment, and
ensure any code that currently reassigns the variable is updated to mutate its
properties instead of reassigning the identifier (apply the same change for the
other occurrence noted near line 404).


value.people.map((x, index) => {
if (x.firstName.length < 3) {
fieldErrors[`people[${index}].firstName`] =
'First name is too short'
}

if (x.lastName.length < 3) {
fieldErrors[`people[${index}].lastName`] =
'Last name is too short'
}
})

if (Object.keys(fieldErrors).length > 0) {
return {
fields: { ...fieldErrors },
}
}

return fieldErrors
},
onChange: ({ value }) => {
let fieldErrors: Record<string, string> = {}

value.people.map((x, index) => {
if (x.firstName.length < 3) {
fieldErrors[`people[${index}].firstName`] =
'First name is too short'
}

if (x.lastName.length < 3) {
fieldErrors[`people[${index}].lastName`] =
'Last name is too short'
}
})

if (Object.keys(fieldErrors).length > 0) {
return {
fields: { ...fieldErrors },
}
}

return fieldErrors
},
},
})
form.mount()

// Since validation runs through the field, a field must be mounted for that array
new FieldApi({ form, name: 'people' }).mount()

await form.replaceFieldValue('people', 0, {
firstName: 'Chuck',
lastName: 'Norris',
})

expect(form.state.values).toStrictEqual({
people: [
{
firstName: 'Chuck',
lastName: 'Norris',
},
],
})
expect.soft(form.state.fieldMetaBase['people']!.errorMap).toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorMap)
.toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorSourceMap)
.toStrictEqual({})
Comment on lines +428 to +452
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add the implementation fix and tighten the regression assertions before merge.

CI is currently red on Lines 446, 449, and 452 because the stale onMount metadata remains after replaceFieldValue. If this PR is intended to merge, please include the production fix; if it is only a repro, keep it non-mergeable/draft.

Also, the test creates lastName errors but only checks firstName, and it does not assert the initial onMount errors exist before replacement.

Optional test tightening after the production fix
     // Since validation runs through the field, a field must be mounted for that array
     new FieldApi({ form, name: 'people' }).mount()
 
+    expect
+      .soft(form.state.fieldMetaBase['people[0].firstName']!.errorMap)
+      .toMatchObject({ onMount: 'First name is too short' })
+    expect
+      .soft(form.state.fieldMetaBase['people[0].lastName']!.errorMap)
+      .toMatchObject({ onMount: 'Last name is too short' })
+
     await form.replaceFieldValue('people', 0, {
       firstName: 'Chuck',
       lastName: 'Norris',
     })
@@
     expect
       .soft(form.state.fieldMetaBase['people[0].firstName']!.errorSourceMap)
       .toStrictEqual({})
+    expect
+      .soft(form.state.fieldMetaBase['people[0].lastName']!.errorMap)
+      .toStrictEqual({})
+    expect
+      .soft(form.state.fieldMetaBase['people[0].lastName']!.errorSourceMap)
+      .toStrictEqual({})
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
form.mount()
// Since validation runs through the field, a field must be mounted for that array
new FieldApi({ form, name: 'people' }).mount()
await form.replaceFieldValue('people', 0, {
firstName: 'Chuck',
lastName: 'Norris',
})
expect(form.state.values).toStrictEqual({
people: [
{
firstName: 'Chuck',
lastName: 'Norris',
},
],
})
expect.soft(form.state.fieldMetaBase['people']!.errorMap).toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorMap)
.toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorSourceMap)
.toStrictEqual({})
form.mount()
// Since validation runs through the field, a field must be mounted for that array
new FieldApi({ form, name: 'people' }).mount()
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorMap)
.toMatchObject({ onMount: 'First name is too short' })
expect
.soft(form.state.fieldMetaBase['people[0].lastName']!.errorMap)
.toMatchObject({ onMount: 'Last name is too short' })
await form.replaceFieldValue('people', 0, {
firstName: 'Chuck',
lastName: 'Norris',
})
expect(form.state.values).toStrictEqual({
people: [
{
firstName: 'Chuck',
lastName: 'Norris',
},
],
})
expect.soft(form.state.fieldMetaBase['people']!.errorMap).toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorMap)
.toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorSourceMap)
.toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].lastName']!.errorMap)
.toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].lastName']!.errorSourceMap)
.toStrictEqual({})
🧰 Tools
🪛 GitHub Actions: PR

[error] 446-446: Vitest failed: expected { onMount: undefined } to strictly equal {}.


[error] 449-449: Vitest failed: expected { onMount: 'First name is too short' } to strictly equal {}.


[error] 452-452: Vitest failed: expected { onMount: 'form' } to strictly equal {}.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/form-core/tests/FormApi.spec.ts` around lines 428 - 452, The failing
test shows stale onMount metadata and error entries remain after calling
FormApi.replaceFieldValue; update the production implementation of
replaceFieldValue (the method named replaceFieldValue on the FormApi class) to
purge/refresh any onMount-related metadata and error maps for the replaced array
field and all nested child keys (e.g., clear entries in form.state.fieldMetaBase
for keys like 'people[0].*' and remove related errorMap/errorSourceMap/onMount
flags) whenever elements are replaced or shifted; after that, tighten the test
by first asserting the initial onMount errors exist for both firstName and
lastName before calling replaceFieldValue and then assert both firstName and
lastName errorMap/errorSourceMap are cleared afterwards (the test touches
form.mount, new FieldApi({ form, name: 'people' }).mount(), replaceFieldValue,
and checks fieldMetaBase/errorMap/errorSourceMap).

})

it("should run onChange validation when inserting an array field's value", () => {
const form = new FormApi({
defaultValues: {
Expand Down
51 changes: 51 additions & 0 deletions packages/form-core/tests/standardSchemaValidator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,4 +608,55 @@ describe('standard schema validator', () => {
])
})
})

it("should clean onMount errors when replacing an array field's value", async () => {
const schema = z.object({
people: z.array(
z.object({
firstName: z.string().min(3),
lastName: z.string().min(3),
}),
),
})

const form = new FormApi({
defaultValues: {
people: [
{
firstName: '',
lastName: '',
},
],
},
validators: {
onMount: schema,
onChange: schema,
},
})
form.mount()

// Since validation runs through the field, a field must be mounted for that array
new FieldApi({ form, name: 'people' }).mount()

await form.replaceFieldValue('people', 0, {
firstName: 'Chuck',
lastName: 'Norris',
})

expect(form.state.values).toStrictEqual({
people: [
{
firstName: 'Chuck',
lastName: 'Norris',
},
],
})
expect.soft(form.state.fieldMetaBase['people']!.errorMap).toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorMap)
.toStrictEqual({})
expect
.soft(form.state.fieldMetaBase['people[0].firstName']!.errorSourceMap)
.toStrictEqual({})
})
})
Loading