1

I am currently playing around with the new React Hooks feature, and I have run into an issue where the state of a functional component is not being updated even though I believe I am doing everything correctly, another pair of eyes on this test app would be much appreciated.

import React, { useState, useEffect } from 'react';

const TodoList = () => {
  let updatedList = useTodoListState({ 
    title: "task3", completed: false 
  });
const renderList = () => {
  return (
    <div>
      {
        updatedList.map((item) => {
          <React.Fragment key={item.title}>
            <p>{item.title}</p>
          </React.Fragment>
        })
      }
    </div>
  )
}
return renderList();
}

function useTodoListState(state) {
  const [list, updateList] = useState([
    { title: "task1", completed: false },
    { title: "task2", completed: false }
  ]);
  useEffect(() => {
    updateList([...list, state]);
  })
  return list;
}

export default TodoList;

updateList in useTodoListState's useEffect function should be updating the list variable to hold three pieces of data, however, that is not the case.

Ayaz Uddin
  • 105
  • 2
  • 13

1 Answers1

1

You have a few problems here:

  1. In renderList you are misusing React.Fragment. It should be used to wrap multiple DOM nodes so that the component only returns a single node. You are wrapping individual paragraph elements each in their own Fragment.
  2. Something needs to be returned on each iteration of a map. This means you need to use the return keyword. (See this question for more about arrow function syntax.)
  3. Your code will update infinitely because useEffect is updating its own hook's state. To remedy this, you need to include an array as a second argument in useEffect that will tell React to only use that effect if the given array changes.

Here's what it should all look like (with some reformatting):

import React, { useState, useEffect } from 'react';

const TodoList = () => {
  let updatedList = useTodoListState({
    title: "task3", completed: false
  });

  return (
    <React.Fragment> // #1: use React.Fragment to wrap multiple nodes
      {
        updatedList.map((item) => {
          return <p key={item.title}>{item.title}</p> // #2: return keyword inside map with braces
        })
      }
    </React.Fragment>
  )
}

function useTodoListState(state) {
  const [list, updateList] = useState([
    { title: "task1", completed: false },
    { title: "task2", completed: false }
  ]);

  useEffect(() => {
    updateList([...list, state]);
  }, [...list]) // #3: include a second argument to limit the effect

  return list;
}

export default TodoList;

Edit:
Although I have tested that the above code works, it would ultimately be best to rework the structure to remove updateList from useEffect or implement another variable to control updating.

Adam
  • 3,829
  • 1
  • 21
  • 31
  • will using `[...list]` as second arg to `useEffect` have different behaviour to just using `[list]`? – lecstor Mar 04 '19 at 06:43
  • @lecstor Yes. The second argument should contain any values from the outer scope that are used inside `useEffect`. Here, the list array is being spread, so the list array in the effect limiter needs to be spread as well. – Adam Mar 04 '19 at 06:51
  • hmm.. It's more that `useEffect` will only run if any values in that array have changed (except the first time), so I don't see how there would be any difference. Unless of course you mutated the list, changing one of it's items without changing the actual list object which is not what `updateList` would do. – lecstor Mar 04 '19 at 07:03
  • I agree with that, but when I tested this, the update ran infinitely without array spread. – Adam Mar 04 '19 at 07:06
  • hmm.. I get `Warning: The final argument passed to useEffect changed size between renders. The order and size of this array must remain constant.` https://codesandbox.io/s/9lw4wo1jqr – lecstor Mar 04 '19 at 07:21
  • and, yeh, it killed my codesandbox without the spread, but that's because `list` is being updated by `useEffect` so it keeps getting triggered. It's just the nature of the test app that is wrong. – lecstor Mar 04 '19 at 07:24
  • @ayaz-uddin, I'd say, for the current implementation, an empty array should be used as the second arg to `useEffect`. (for a rather different implementation see https://codesandbox.io/s/wow5q2k9kk) – lecstor Mar 04 '19 at 07:44
  • Ultimately, it would be better to either implement another variable to control updating or rework the structure to remove updating from `useEffect` altogether. – Adam Mar 04 '19 at 12:54