Why does forEach loop only set the last value if finds to state. ReactJS-CodePudding

const CategoriesData = [
  {
    name: "Category1",
    isActive: true,
    children: [
      {
        name: "Category1Child",
        isActive: false,
      }
    ]
  },
  {
    name: "Category2",
    isActive: false,
  },
  {
    name: "Category3",
    isActive: true,
    children: [
      {
        name: "Category3Child",
        isActive: false,
      }
    ]
  }
];

const [disabledCategories, setDisabledCategories] = useState([]);

function notActiveCategories(categories) {
  // Loop logs out at least 7 isActive: false categories.
  categories.forEach((category) => {
    if (category.isActive) notActiveCategories(category.children);
    if (!category.isActive) {
      setDisabledCategories([...disabledCategories, category]);
      console.log(category);
    }
  });
};

useEffect(() => {
  notActiveCategories(CategoriesData);
  console.log(disabledCategories); // Only 1 category is in the array.
}, []);

I feel like the function the loop is in calling itself is causing the disabledCategories state to revert to when it was empty and that is leading to only the last step of the foreach to be set.

So how would i get this to loop through the categories array and have the disabledCategories state to contain all of the category objects that have isActive: false. Which in the example of CategoriesData above, it would mean that the disabledCategories state would contain:

[
  {
    name: "Category1Child",
    isActive: false,
  },
  {
    name: "Category2",
    isActive: false,
  },
  {
    name: "Category3Child",
    isActive: false,
  },
];

CodePudding user response：

Try changing your setDisabledCategories to use the previous state param that comes from setState:

setDisabledCategories(prevState => [...prevState, category])

When multiple setState calls are batched together you need to be careful so they don't override each other. Using this method ensures that your setState calls are "chained" so you always get the updated state.

CodePudding user response：

Way 1: Affect after recursive loop

function notActiveCategoriesRecusive(categories) {
  let notActive = []
  categories.forEach((category) => {
    if (category.isActive) notActive = [...notActive, ...(notActiveCategories(category.children))];
    if (!category.isActive) {
      notActive.push(category)
    }
  });
  return notActive
};

function notActiveCategories(categories) {
   setDisabledCategories(notActiveCategoriesRecusive(categories)
}

Way 2: Get the last state because it doesn't has time to refresh

function notActiveCategories(categories) {
  categories.forEach((category) => {
    if (category.isActive) notActiveCategories(category.children);
    if (!category.isActive) {
      setDisabledCategories(oldState => ([...oldState, category]))
    }
  });
};

CodePudding user response：

I'd only call setState once with the filtered array:

const findInactive = data =>
  data.filter(e => !e.isActive)
    .concat(...data.filter(e => e.children)
                   .map(e => findInactive(e.children)))
;

const categoriesData = [ { name: "Category1", isActive: true, children: [ { name: "Category1Child", isActive: false, } ] }, { name: "Category2", isActive: false, }, { name: "Category3", isActive: true, children: [ { name: "Category3Child", isActive: false, } ] } ];

const inactive = findInactive(categoriesData)
// the following is neeeded if it's possible for a
// node to have children and be inactive
//  .map(({name, isActive}) => ({name, isActive}))
;
console.log(inactive);
//setDisabledCategories(inactive); // one time in React

This makes the code a lot easier to reason about and decouples React's API out from the filtering logic, which can be moved out to a generic function agnostic of React.

As others have mentioned, if you do want to call setState multiple times as a batch update, you can use the prevState callback to chain the updates: setDisabledCategories(prevState => [...prevState, category]);.