1

In my app, I have a RoomsContainer parent component, which has an array of Room child components. In Room, I want there to be a button to add additional Room elements to the RoomsContainer by calling the parent component's addRoom function.

This is what I have:

function RoomsContainer() {
  const [rooms, setRooms] = useState([]);

  const addRoom = () => {
    var uniqid = require('uniqid');
    let key = uniqid("room-");
    setRooms([
      ...rooms,
      <Room
        key={key}
        id={key}
        addRoom={addRoom}
        removeRoom={removeRoom}
      />
    ]);
    console.log(rooms);
  }

  useEffect(() => {
    addRoom();
  }, []);

  // ...

  return (
    <div id="room-wrapper">
      {rooms}
    </div>
  )
}

function Room(props) {
  // ...
  return (
    <button onClick={props.addRoom}>
      Add room
    </button>
  )
}

This adds the first room on page load, however when I click on the Add Room button in the Room component, it doesn't add any additional rooms. Clicking it also appears to re-render the entire RoomsContainer component completely with a new Room element instead of modifying its state. Also console.log(rooms) always displays an empty array, even right after setting it.

Mike
  • 23,542
  • 14
  • 76
  • 87

2 Answers2

1

You have a stale closure in your addRoom function and I don't think you should save jsx to local state. The following should work:

const id = ((id) => () => ++id)(0);

function RoomsContainer() {
  const [rooms, setRooms] = React.useState([]);
  //not sure why you want to use require in a react app
  //var uniqid = require('uniqid');

  //use React.useCallback so this function never changes
  const addRoom = React.useCallback(
    //setRooms takes a callback function that receives the current rooms
    //  only store the id in rooms, not the jsx
    () => setRooms((rooms) => [...rooms, id()]),
    []
  );
  const removeRoom = React.useCallback(
    //setRooms takes a callback function that receives the current rooms
    //  only store the id in rooms, not the jsx
    (id) =>
      setRooms((rooms) =>
        rooms.filter((room) => room !== id)
      ),
    []
  );

  React.useEffect(() => {
    addRoom();
  }, [addRoom]);

  return (
    // do not use id in html elements, id should be unique
    // in the DOM but you can't/shouldn't guarantee that your app doesn't
    // have multiple RoomContainer elements
    <ul>
      {rooms.map((id) => (
        <Room
          key={id}
          id={id}
          addRoom={addRoom}
          removeRoom={removeRoom}
        />
      ))}
    </ul>
  );
}
//make Room a pure component so there are no needless re renders
const Room = React.memo(function Room({
  addRoom,
  removeRoom,
  id,
}) {
  return (
    <li>
      <button onClick={addRoom}>Add room</button>
      <button onClick={() => removeRoom(id)}>
        Remove room {id}
      </button>
    </li>
  );
});

ReactDOM.render(
  <RoomsContainer />,
  document.getElementById('root')
);
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.4/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.4/umd/react-dom.production.min.js"></script>
<div id="root"></div>
HMR
  • 37,593
  • 24
  • 91
  • 160
1

It's normal for console.log(rooms); to show an empty array initially, cause setting state is asynchronous.

This is what you should do:

function RoomsContainer() {
  const [rooms, setRooms] = useState([]);

  const addRoom = useCallback(() => {
    var uniqid = require("uniqid");
    let key = uniqid("room-");
    setRooms(rooms => [...rooms, key]);
  }, []);

  useEffect(() => {
    addRoom();
  }, []);

  return (
    <div id="room-wrapper">
      {rooms.map((key) => {
        return <Room key={key} id={key} addRoom={addRoom} />;
      })}
    </div>
  );
}
Joshua
  • 589
  • 1
  • 5
  • 19
  • If you use useCallback you should not make it needlessly dependent on `rooms` because it will needlessly re create the funcion every time `rooms` changes and re render all the items in the list. You can remove `rooms` from the dependency if you set state like this: `setRooms(rooms=>[...rooms, key]);` – HMR Nov 12 '20 at 21:17