-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
Moving subfields of array field mix their values #692
Comments
I think this is a bug with our |
If it saves a little time, after some research, I wrote a test to check whether there was a problem with the moveValue function, but the test went well. I wonder if the problem isn't the collision between an uncontrolled and a controlled field, and that screws up the ui. it('should correctly move an array value, even with undefined values', () => {
const form = new FormApi({
defaultValues: {
users: [
{
name: undefined,
},
{
name: 'one',
},
{ name: 'two' },
{
name: 'three',
},
{
name: undefined,
},
],
},
})
const field = new FieldApi({
form,
name: 'users',
})
field.moveValue(3, 4)
expect(field.getValue()).toStrictEqual([
{
name: undefined,
},
{ name: 'one' },
{ name: 'two' },
{
name: undefined,
},
{
name: 'three',
},
])
}) |
Hey @Balastrong, was this fixed when you updated your |
Unfortunately I think there's still something not behaving here. I made a short example: https://stackblitz.com/edit/tanstack-form-grdmqr?file=src%2Findex.tsx&preset=node First issue is that indeed having The second issue is that if we use the move button and then we submit, the old value "one" remains in the input, but again this is probably still related to undefined being put there. |
This test on Failing Test
it('should correctly move an array value, even with undefined values and not alter the state after submit', async () => {
function Comp() {
const form = useForm({
defaultValues: {
people: [
{
name: undefined,
},
{
name: 'one',
},
],
},
})
return (
<div>
<form
onSubmit={(e) => {
e.preventDefault()
e.stopPropagation()
form.handleSubmit()
}}
>
<form.Field name="people" mode="array">
{(field) => {
return (
<div>
{field.state.value.map((_, i) => {
return (
<form.Field key={i} name={`people[${i}].name`}>
{(subField) => {
return (
<div>
<label>
<div>Name for person {i}</div>
<input
value={subField.state.value ?? ''}
onChange={(e) =>
subField.handleChange(e.target.value)
}
/>
</label>
</div>
)
}}
</form.Field>
)
})}
<button type="button" onClick={() => field.moveValue(1, 0)}>
Move 1 to 0
</button>
</div>
)
}}
</form.Field>
<form.Subscribe
selector={(state) => [state.canSubmit, state.isSubmitting]}
children={([canSubmit, isSubmitting]) => (
<>
<button type="submit" disabled={!canSubmit}>
{isSubmitting ? '...' : 'Submit'}
</button>
</>
)}
/>
</form>
</div>
)
}
const { getByText, findByLabelText, queryByText, findByText } = render(
<Comp />,
)
const input = await findByLabelText('Name for person 0')
expect(input).toBeInTheDocument()
expect(input).toHaveValue('')
const input2 = await findByLabelText('Name for person 1')
expect(input2).toBeInTheDocument()
expect(input2).toHaveValue('one')
await user.click(getByText('Move 1 to 0'))
expect(input).toHaveValue('one')
expect(input2).toHaveValue('')
await user.click(await findByText('Submit'))
expect(input).toHaveValue('one')
expect(input2).toHaveValue('') // This gets value 'one'
}) |
I have run into this similar, if not same problem. I have an array of objects that represent other sub-fields of various depths. The internals of Once that was worked around, I noticed that if one of the sub-fields that has an If I do not render the I think the problem is that while the internal form store state and field instance states reflect the desired overall state of the form, the EDIT: I'm on |
Describe the bug
When the input is still idle (I mean undefined) and you try to change its position it takes the value of other subfields. It messes things up.
Codesandbox
https://codesandbox.io/p/sandbox/focused-pond-s96spf?file=%2Fsrc%2FApp.js%3A169%2C8
Steps to reproduce
Expected behavior
I expect it to keep its value.
The text was updated successfully, but these errors were encountered: