2

I'm creating a timer app built using react-hooks and an array of this timers I don't understand why timerList changes

Here it is the parent component

const [timerList, setTimerList] = useState([]);


const removeTimer = () => {
  console.log("timerList", timerList);
};

return (
    <div id="main">
      {timerList ? timerList.map((child) => child) : null}
      <div className="add-button before">
        <button
          onClick={() => {
            const time = new Date();
            time.setSeconds(time.getSeconds() + 0);
            setTimerList((timerList) => [
              ...timerList,
              <FullTimer
                expiryTimestamp={time}
                removeTimer={() => {
                  removeTimer();
                }}
                id={window.prompt("Insert timer name") + ` ${timerList.length}`}
                key={timerList.length}
              />,
            ]);
          }}
        >

The interested child's component part:

<button
  onClick={() => {
     removeTimer();
  }}
>

The child component is a custom timer with some css, and when i call removeTimer the value of timerList (in the parent component) changes, when it should remain the same.

What am I missing?

P.S. the button tags aren't closed because i have some element inside them that use awesome-font

Gino_P
  • 74
  • 7

3 Answers3

2

Side note: In general it's considered bad practice to store components in another components state.


But that's not really the problem here. Given your code, it's a simple closure problem.

This:

const removeTimer = () => {
  console.log("timerList", timerList);
};

definition closes over the current timerList. So it will log it, as it was when removeTimer was assigned. Currently that's on every render. So it will log the state seemingly one step behind. There's no fix around that, because that's just how closures work.

Provided you actually want to remove a timer, when removeTimer is invoked, you would need to use the callback version of the updater (setTimerList) and pass some identifying value so that you can actually remove the correct one.

This would all be a lot simpler, if you followed the initial advice and don't store the component in the state, but rather it's defining properties.


The following would be a working example (please excuse my typescript):

import React, { useState } from 'react';

type FullTimerProps = {
  id: string;
  expiryTimestamp: Date;
  removeTimer: () => void;
}

const FullTimer = ({expiryTimestamp, removeTimer, id}: FullTimerProps): JSX.Element => {
  return (
    <div>
      <button onClick={removeTimer}>remove</button>
      {id}: {expiryTimestamp.toLocaleDateString()}
    </div>
  );
};

type Timer = {
  id: string;
  expiryTimestamp: Date;
};

const TimerList = (): JSX.Element => {
  const [timerList, setTimerList] = useState<Timer[]>([]);

  const removeTimer = (timer: Timer) => {
    setTimerList(timerList => timerList.filter(t => t.id !== timer.id));
  };

  return (
    <div id="main">
      {timerList.map(timer => (
        <FullTimer
          key={timer.id}
          id={timer.id}
          expiryTimestamp={timer.expiryTimestamp}
          removeTimer={() => removeTimer(timer)}
        />
      ))}

      <div className="add-button before">
        <button
          onClick={() =>
            setTimerList(timerList => [...timerList, {
              id: window.prompt('Insert timer name') + ` ${timerList.length}`,
              expiryTimestamp: new Date()
            }])}
        >Add
        </button>
      </div>
    </div>
  );
};
Yoshi
  • 54,081
  • 14
  • 89
  • 103
1

changing this code snippet

            setTimerList((timerList) => [
              ...timerList,
              <FullTimer
                expiryTimestamp={time}
                removeTimer={() => removeTimer()}
                id={window.prompt("Insert timer name") + ` ${timerList.length}`}
                key={timerList.length}
              />,
            ]);

to

            timerList.push(<FullTimer
              expiryTimestamp={time}
              removeTimer={() => removeTimer()}
              id={window.prompt("Insert timer name") + ` ${timerList.length}`}
              key={timerList.length}
            />);
            setTimerList([...timerList]);

Fixed the problem you are having. Although this change is not recommended because it is not immutable approach, but it fixes this case.

UPDATE: It turned out that you duplicated the removeTimer function during the setTimerList call which cause the child component to capture the timerList at the moment of assignment. Which is mentioned at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures?retiredLocale=vi#closure as mr @yoshi has shown

Phạm Huy Phát
  • 783
  • 9
  • 17
0

Try to write your onclick function like this

<button
  onClick={() => removeTimer()}
>

Also over here

 <FullTimer
    expiryTimestamp={time}
    removeTimer={() => removeTimer()}
Neel Dsouza
  • 1,342
  • 4
  • 15
  • 33
  • same behaviour. When i click the button in a component timerLIst is seen as the original list without the next components (included the component's button). Ex: if i click the button in the first component, timerLIst === [] second component timerList === [ {...} ] and go on... – Gino_P Aug 12 '21 at 10:18
  • Is it possible to add a stackblitz code so that it will better for us to understand ur problem – Neel Dsouza Aug 12 '21 at 11:58
  • [link to stackblitz](https://stackblitz.com/edit/react-jybhsx?file=src/App.js) Idk if i missed something because it's the first time i use it – Gino_P Aug 12 '21 at 12:30
  • I was expecting a workable code where the issue can be reproduced. – Neel Dsouza Aug 12 '21 at 13:59
  • I really don't know why it isn't working. here it is the [github repo](https://github.com/ginop-1/react-timer-app) with all the files, if it helps. – Gino_P Aug 12 '21 at 14:18
  • @Gino_P interesting case, I am checking it rightnow using your github repo – Phạm Huy Phát Aug 13 '21 at 05:00