4
\$\begingroup\$

I have declared a react hook which will contain an array of object as follows:

const [rowDataTracker, setRowDataTracker] = useState([]);

Now I need to update the hook to basically create a payload as the following:

[
  {id: 100, payload: {field1: "value", field2: "value"}}, 
  {id: 105, payload: {field1: "value", field3: "value"}},
  {id: 109, payload: {field4: "value"}}
]

I need to check the following condition Before updating the hook:

  • Need to first check if the id exists in the array or not
  • if the id exists, then need to update/add the filed to the payload object
  • if the id does not exist, add a new object to the array with id and the field

const ifItemExist = (arr, item) => {
  return arr.findIndex((e) => e.id === item);
};
function storeEdit(rowId, field, newValue) {
  let itemIndex = ifItemExist(rowDataTracker, rowId);
  let newArr = [...rowDataTracker];
  if (itemIndex > -1) {
    newArr[itemIndex]["payload"][columnId] = newValue;
  } else {
    const obj = {
      itemNbr: rowId,
      payload: {
        [field]: newValue
      }
    };
    newArr.push(obj);
  }
  setRowDataTracker(newArr);
}

I am not sure if this is the correct way to update a hook. Any review and suggestion will be appreciated.

New contributor
kukuha is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
5
\$\begingroup\$

Mutation You are mutating the existing state when you do

let newArr = [...rowDataTracker];
// ..
  newArr[itemIndex]["payload"][columnId] = newValue;

because spread syntax only creates a shallow copy of the array or object. This should never be done in React. (See end of answer for how to fix it - create the new obj no matter what, then either push it to the new array, or replace the existing object in the array, without mutating the existing object in the array)

Dot notation Most people prefer to read and write using dot notation in JS when possible - it's more concise and easier to read. ESLint rule: dot-notation. You can use .payload instead of ["payload"].

Property/variable naming The rowId refers to the same thing as the itemNbr, which looks to refer to the same thing as the id in the rowDataTracker array. It's good to be consistent with variable names and properties when possible - consider picking one and sticking with it. If you use the variable name itemNbr, you can use it shorthand in the object literal:

const obj = {
  itemNbr,
  // ...

id or itemNbr or both? You check against the id property in rowDataTracker in ifItemExist, but you push to the same array with a property named itemNbr. This looks very likely to be a typo. You probably meant to push an object with an id property, or you wanted to check against the itemNbr property instead.

Misleading function name ifItemExist doesn't return a boolean indicating if an item exists - it returns the index of the possibly-existing item. Maybe call it getItemIndex.

Prefer const over let when the variable name doesn't need to be reassigned.

In all:

const getItemIndex = (arr, item) => {
  return arr.findIndex((e) => e.id === item);
};

function storeEdit(id, field, newValue) {
  const itemIndex = getItemIndex(rowDataTracker, id);
  const obj = {
    id,
    payload: {
      ...rowDataTracker[itemIndex]?.payload,
      [field]: newValue
    }
  };
  if (itemIndex === -1) {
    setRowDataTracker([...rowDataTracker, obj]);
    return;
  }
  const newArr = [...rowDataTracker];
  newArr[itemIndex] = obj;
  setRowDataTracker(newArr);
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you for the detailed review. It is helpful. \$\endgroup\$ – kukuha Sep 17 at 4:27

Your Answer

kukuha is a new contributor. Be nice, and check out our Code of Conduct.

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.