Skip to content

fix(form-core, react-form): remove overwritting of falsy values upon array element shifting #1440

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 9 commits into
base: main
Choose a base branch
from
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
13 changes: 10 additions & 3 deletions packages/form-core/src/FieldApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1175,9 +1175,16 @@
// The name is dynamic in array fields. It changes when the user performs operations like removing or reordering.
// In this case, we don't want to force a default value if the store managed to find an existing value.
if (nameHasChanged) {
this.setValue((val) => (val as unknown) || defaultValue, {
dontUpdateMeta: true,
})
this.setValue(
(val) => {

Check warning on line 1179 in packages/form-core/src/FieldApi.ts

View check run for this annotation

Codecov / codecov/patch

packages/form-core/src/FieldApi.ts#L1178-L1179

Added lines #L1178 - L1179 were not covered by tests
// Preserve falsy values used for checkboxes or textfields (e.g. false, '')
const newValue = val !== undefined ? val : defaultValue
return newValue

Check warning on line 1182 in packages/form-core/src/FieldApi.ts

View check run for this annotation

Codecov / codecov/patch

packages/form-core/src/FieldApi.ts#L1182

Added line #L1182 was not covered by tests
},
{
dontUpdateMeta: true,
},
)
} else if (defaultValue !== undefined) {
this.setValue(defaultValue as never, {
dontUpdateMeta: true,
Expand Down
4 changes: 2 additions & 2 deletions packages/form-core/src/FormApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,7 @@ export class FormApi<
const errorMapKey = getErrorMapKey(validateObj.cause)

for (const field of Object.keys(
this.state.fieldMeta,
this.baseStore.state.fieldMetaBase, // Iterate over the field meta base since it is the source of truth
) as DeepKeys<TFormData>[]) {
const fieldMeta = this.getFieldMeta(field)
if (!fieldMeta) continue
Expand Down Expand Up @@ -1569,7 +1569,7 @@ export class FormApi<
const errorMapKey = getErrorMapKey(validateObj.cause)

for (const field of Object.keys(
this.state.fieldMeta,
this.baseStore.state.fieldMetaBase, // Iterate over the field meta base since it is the source of truth
) as DeepKeys<TFormData>[]) {
const fieldMeta = this.getFieldMeta(field)
if (!fieldMeta) continue
Expand Down
15 changes: 14 additions & 1 deletion packages/react-form/src/useField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,20 @@ export const Field = (<
const fieldApi = useField(fieldOptions as any)

const jsxToDisplay = useMemo(
() => functionalUpdate(children, fieldApi as any),
() => {
/**
* When field names switch field store and form store are out of sync.
* When in this state, React should not render the component
*/
const isFieldStoreOutofSync =
fieldApi.state.value !== fieldApi.form.getFieldValue(fieldOptions.name)

if (isFieldStoreOutofSync) {
return null
}

return functionalUpdate(children, fieldApi as any)
},
/**
* The reason this exists is to fix an issue with the React Compiler.
* Namely, functionalUpdate is memoized where it checks for `fieldApi`, which is a static type.
Expand Down
99 changes: 99 additions & 0 deletions packages/react-form/tests/useForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -794,4 +794,103 @@ describe('useForm', () => {

expect(fn).toHaveBeenCalledTimes(1)
})

it('preserves empty string values when removing array elements', async () => {
function Comp() {
const form = useForm({
defaultValues: {
interests: [{ interestName: '', id: 0 }],
},
})
return (
<form>
<form.Field name="interests" mode="array">
{(interestsFieldArray) => (
<div>
<button
type="button"
data-testid="add-interest"
onClick={() => {
interestsFieldArray.pushValue({
id: form.getFieldValue('interests').length,
interestName: '',
})
}}
>
Add
</button>

{interestsFieldArray.state.value.map((row, i) => {
return (
<form.Field
key={row.id}
name={`interests[${i}].interestName`}
>
{(field) => {
return (
<div style={{ display: 'flex' }}>
<input
type="text"
value={field.state.value}
onChange={(e) => {
field.handleChange(e.target.value)
}}
/>
<button
type="button"
onClick={() => {
interestsFieldArray.removeValue(i)
}}
data-testid={`remove-interest-${i}`}
>
Remove
</button>
</div>
)
}}
</form.Field>
)
})}
<div data-testid="interests-log">
<p>{JSON.stringify(form.getFieldValue('interests'))}</p>
</div>
<button
type="button"
data-testid="append-interests-to-paragraph"
onClick={() => {
const interestsLog = document.querySelector(
'[data-testid="interests-log"] p',
)
if (interestsLog) {
interestsLog.textContent = JSON.stringify(
form.getFieldValue('interests'),
)
}
}}
>
Log Interests
</button>
</div>
)}
</form.Field>
</form>
)
}

const { getByTestId } = render(<Comp />)

// Add 2 interests
await user.click(getByTestId('add-interest'))
await user.click(getByTestId('add-interest'))

// Remove the first interest
await user.click(getByTestId('remove-interest-0'))

expect(getByTestId('interests-log').textContent).toBe(
JSON.stringify([
{ id: 1, interestName: '' },
{ id: 2, interestName: '' },
]),
)
})
})