automatic-azure

Removing an entry from an array causes the array to receive invalid data

Hi there, I have some role management code where I iterate through an array of roles from the backend and show checkboxes for each role and each role is linked to an index in a form array. Whenever I uncheck a checkbox it seems to correctly call form.removeValue (with the correct index) but then when I check the form.state.values.roles there is a weird entry of whatever the last entry of the initial array was but now with just the name property of the whole Role object, which in turn causes my zod validation to fail. I have a stackblitz where this odd behaviour can be reproduced: https://stackblitz.com/edit/tanstack-form-9vub96pu?file=src%2Findex.tsx Assuming when I'm editing the user which has both Admin and User: 1. I click Admin to remove it 1. In the breakpoint I have on the onChange I correctly see that event.target.checked is false and the else branch is executed 1. When logging form.state.values at this point I see the correct data 1. It correctly finds the id of the role to remove 1. field.removeValue(roleIdx) seems to be called with the correct value 1. The form state now results in

{ "name": "John Doe", "age": 30, "roles": [ { "name": "User", "external": false }, { "name": "User" } ] }

{ "name": "John Doe", "age": 30, "roles": [ { "name": "User", "external": false }, { "name": "User" } ] }

1. Click the Submit button This then trips up Zod which says that roles[1].external is required. What I find particularly notable is that as far as I can tell this problem is only happening when editing an existing user, but when I log the whole form.state.values as soon as the form loads with the existing data that looks a-ok. So the question is, why is that second entry in the array even there? What is adding it? Especially since it's not even the role I just clicked to remove.

Jeroen Claassens

StackBlitz

Form Array Example (duplicated) - StackBlitz

Run official live example code for Form Array, created by Tanstack on StackBlitz

35 Replies

passive-yellow•3mo ago

It may be related to this issue: https://github.com/TanStack/form/issues/1439

GitHub

Falsy values are being turned into undefined upon removing array fi...

Describe the bug When removing array elements, currently subfields with falsy values on the last row are being changed to undefined. Added a condition to specifically check for undefined. This is c...

passive-yellow•3mo ago

If not, I'd appreciate it if you could create a GitHub issue with the reproduction. You can also share the stackblitz link in the existing issue if it's related.

automatic-azureOP•3mo ago

hm bit hard to tell but it might be yeah. Juan's steps to reproduce mis 1 step in that you have to remove the 0th or 1st entry before pressing log to actually see the behaviour occur and my implementation is slightly different but it might be related. though in my case the property is removed entirely rather than being set to explicitly undefined @juanvilladev I'm just gonna tag you in here. You can probably best say if my problem is related or not since you delved into this.

passive-yellow•3mo ago

that may be because of stringification

passive-yellow•3mo ago

also wow, didn't expect to meet a sapphire developer :PauseChamp: good stuff

automatic-azureOP•3mo ago

oh haha yeah this is for my work project though ^^" trying my damnest to see if I find some workaround because this bug is currently in our production environment as well but other than marking the flasy field as optional in zod which at least would sort of work that doesn't take away the wonky data that ends up in the form state :\

passive-yellow•3mo ago

it's hacky, but perhaps a string enum can save it convert it after submission

automatic-azureOP•3mo ago

~~scratch that I made a mistake still~~

automatic-azureOP•3mo ago

Sadly no. Forked my own stackblitz to experiment and if I start by unchecking User the last entry still gets messed up, or if I uncheck Admin then the incorrect User entry gets added and Admin removed.

index.tsx

automatic-azureOP•3mo ago

Bypassing field methods and using form.setFieldValue also doesn't work :\

const roleIdx = field.state.value.findIndex(
  (stateRole) => stateRole.name === role.name
);
const newFieldValue = field.state.value.filter(
  (_, fIdx) => fIdx !== roleIdx
);
console.log(newFieldValue);
form.setFieldValue('roles', newFieldValue);

const roleIdx = field.state.value.findIndex(
  (stateRole) => stateRole.name === role.name
);
const newFieldValue = field.state.value.filter(
  (_, fIdx) => fIdx !== roleIdx
);
console.log(newFieldValue);
form.setFieldValue('roles', newFieldValue);

This is weirdest of all because this shows that whatever setFieldValue gets as a parameter does not then match with what is logged by field.state.values

passive-yellow•3mo ago

yes, because field meta is not considered when shifting like this removeValue manages meta shifting is roles.name unique?

automatic-azureOP•3mo ago

yes

passive-yellow•3mo ago

you could refactor to a Record<RoleName, RoleData> structure which is accessible through 'roles.${role.name}.external' then call deleteField or setFieldValue to manage state optionally a RoleName[] top-level property that stores the ordering of object keys

automatic-azureOP•3mo ago

hm I'll try those things but it won't be before tomorrow. You also gave me another idea to try so I got some testing to do. correction: monday. Forgot that tomorrow is ascension day and for that same reason I have a day off Friday. So if @juanvilladev can chime in before that, that would be awesome 😄

passive-yellow•3mo ago

I'll probably have some time over the weekend, so I can tak a look at the PR see if there's some way to help

automatic-azureOP•3mo ago

@Luca | LeCarbonator I tried the workaround of using Record<string, Role and the problem persists still that the property disappears :\ https://i.imgur.com/QtWeyOn.png

Imgur

automatic-azureOP•3mo ago

for reference that is with this code. This can be pasted directly into a copy of the stackblitz above.

index.tsx

passive-yellow•3mo ago

am I seeing it right that you're setting the record by filtering externally instead of deleting the field?

automatic-azureOP•3mo ago

sorry I don't quite follow which bit of code you are referring to

passive-yellow•3mo ago

- field.setValue(Object.fromEntries(Object.entries(x).map(/* ... */))
Doesn't manage meta for you
+ form.deleteField(`roles.${entryToRemove}`)

- field.setValue(Object.fromEntries(Object.entries(x).map(/* ... */))
Doesn't manage meta for you
+ form.deleteField(`roles.${entryToRemove}`)

this is what I had in mind with my initial suggestion

automatic-azureOP•3mo ago

oh hm

passive-yellow•3mo ago

the setter will likely work, but I'm petty so I'll suggest using field.handleChange over field.setValue :Bueno: I'll make a stackblitz to give the file a try :PepeThumbs:

automatic-azureOP•3mo ago

No I changed it to

onChange={(event) => {
  console.log(role);
  if (event.target.checked) {
    form.setFieldValue(`roles.${role.name}`, role)
  } else {
    form.deleteField(`roles.${role.name}`)
  }
}}

onChange={(event) => {
  console.log(role);
  if (event.target.checked) {
    form.setFieldValue(`roles.${role.name}`, role)
  } else {
    form.deleteField(`roles.${role.name}`)
  }
}}

and it still behaves the same incorrect way First log is from the onChange and the second log is from pressing the Log button right after

passive-yellow•3mo ago

looks like it's become stale because you're forcing it to use a field that doesn't exist See the line:

            return (
              <>
                {Object.entries(roles).map(

            return (
              <>
                {Object.entries(roles).map(

using field.state.value instead of roles should force a reload which should also not cause errors with nonexistent fields

passive-yellow•3mo ago

https://stackblitz.com/edit/vitejs-vite-6vnpktet?file=src%2FApp.tsx

LeCarbonator

StackBlitz

Vitejs - Vite (duplicated) - StackBlitz

Next generation frontend tooling. It's fast!

automatic-azureOP•3mo ago

that's in the array example on the docs as well yes but I don't think I can do that because in reality the list of roles comes from the backend and there should be a checkbox for every role but if I use field.state.value then it would by default show only those checkboxes for the roles the person already has

passive-yellow•3mo ago

then you need a check field.state.value's keys first you have a disconnect between the user interface (checkboxes, always visible) and the data you want to share (the roles field). so the checkboxes should be rendered from the query data, and based on the clicked checkbox, call field.deleteField or field.handleChange TL;DR your checkbox should not be inside a form.Field as it doesn't actually use that data in any way. The field is completely dependent on its parent field (roles) here you go. Stackblitz updated. Basically a compound field :PepeThumbs: I‘ll sketch why this unintuitive behaviour happened, but since you have this bug in prod, the stackblitz should help you fix that first

automatic-azureOP•3mo ago

hm that does work then yes. Now I just have to figure out if I can revert it back to an array of roles to save on some back-and-forth parsing but this already puts me on the right track.

passive-yellow•3mo ago

yeah, could be that you just encountered this issue when creating the previous array method

automatic-azureOP•3mo ago

Think I got it, that wasn't too hard of a de-refactor and this onChange works:

onChange={(event) => {
  if (event.target.checked) {
    field.handleChange((prev) => [...prev, role]);
  } else {
    field.handleChange((prev) => prev.filter((p) => p.name !== role.name));
  }
}}

onChange={(event) => {
  if (event.target.checked) {
    field.handleChange((prev) => [...prev, role]);
  } else {
    field.handleChange((prev) => prev.filter((p) => p.name !== role.name));
  }
}}

well it's been quite the journey but I'm glad we got there in the end. Thank you so very much Luca 🙏

passive-yellow•3mo ago

hopefully this makes some sense

passive-yellow•3mo ago

this could cause some meta mismatches actually. If role.nameis at index 1, then index 0 would have a wrongly shifted meta. There's helper functions for array manipulation, most notably field.removeValue() and field.pushValue (or field.insertValue) since you pass a whole new array, it won't know where the existing indeces ended up

automatic-azureOP•3mo ago

ah yeah right of course so like this

onChange={(event) => {
  if (event.target.checked) {
    field.pushValue(role);
  } else {
    const index = field.state.value.findIndex(
      (stateRole) => stateRole.name === role.name
    );
    field.removeValue(index);
  }
}}

onChange={(event) => {
  if (event.target.checked) {
    field.pushValue(role);
  } else {
    const index = field.state.value.findIndex(
      (stateRole) => stateRole.name === role.name
    );
    field.removeValue(index);
  }
}}

passive-yellow•3mo ago

should be right, yeah filterValues will come soon :Bueno: it's in a PR as of now

automatic-azureOP•3mo ago

on a similar related note I should probably make my React fragment use a key of role.name instead of array index just to be on the safe side and yes this is clear. Tyvm!

Gaming

Programming

Removing an entry from an array causes the array to receive invalid data

Did you find this page helpful?