Jelmer
Jelmer

Reputation: 353

State not referenced as I expect

While trying to build a React Component that renders a list of objects I ran into some behaviour that I cannot explain.

I use the useState hook to keep track of a list of animals. When I click the button 'add animal', I add an object to that list containing a random animal. I do this several times.

...so far so good, an object is created and added every time, and my animalList is properly rendered.

But here is where I get lost;

When I click the 'Remove' button on an Animal component it logs the animalList to the console, but it shows a different value for every item in the list, while I expect it to be the same for all of them.

It appears that the value of animalList is equal to what it was at the time the object was created rather than referencing the state.

My next step would be to remove the clicked object, but they don't seem to share the same reference to the list.

Can somebody help me understand what is happening here? I have added the code required to recreate the issue:

import { useState } from 'react';
import './App.css';

export default function App() {
  const [animalList, updateAnimalList] = useState([]);

  const animals = [
    {name: 'anaconda'}, {name: 'brachiosaurus'}, {name: 'chimpansee'}, {name: 'dragon'}, {name: 'eagle'}, {name: 'fox'},
    {name: 'giraffe'}, {name: 'hellhound'}, {name: 'iguana'}, {name: 'jackal'}, {name: 'koala'}, {name: 'lion'},
    {name: 'meerkat'}, {name: 'nyan-cat'}, {name: 'ostrich'}, {name: 'pterodactyl'}, {name: 'quail'}, {name: 'rhinoceros'},
    {name: 'sfinx'}, {name: 'triceratops'}, {name: 'unicorn'}, {name: 'vampire deer'}, {name: 'whale'}, {name: 'xiao'},
    {name: 'yoghurt'}, {name: 'zebra'},
  ];

  const addAnimal = () => {
    updateAnimalList([...animalList,
      { ...animals[Math.floor(Math.random() * animals.length)],
        onClick: removeAnimal,
      }
    ]);
  }

  const removeAnimal = () => {
    console.log(animalList);
    // let newArray = [...animalList];
    //newArray.splice(index, 1);
    //updateAnimalList(animalList);
  };

  return (
    <div className="app">

        <button onClick={addAnimal}>addAnimal</button>
        { animalList.map( (animal, index) => {
          return (
            <Animal key={index} {...animal} />
          )
        })}

    </div>
  );
};

export function Animal(animal) {
  return (
    <div className="card">
      <h2>{ animal.name }</h2>
      <button onClick={animal.onClick}>Remove</button>
    </div>
  )
}

Upvotes: 1

Views: 98

Answers (4)

Giorgi Moniava
Giorgi Moniava

Reputation: 28685

Because every time you add an animal here:

 { ...animals[Math.floor(Math.random() * animals.length)],
        onClick: removeAnimal,
 }

you are also storing a function reference (removeAnimal) in the state.

The version of removeAnimal which you are storing is from the render when the click happened (aka stale closure). Hence, inside removeAnimal:

let newArray = [...animalList];

the animalList is also from the render when the click happened.

No reason to store removeAnimal on each click inside array. Just declare it as function and pass an id of object you want to delete. Then you can always use that single function.

Also you seem to be using index as key which is not recommended especially if array items may reorder. Use some id instead.

So you could do:

{
  animalList.map((animal) => {
    return (
      <Animal
        key={animal.id}
        {...animal}
        onClick={() => removeAnimal(animal.id)}
      />
    );
  });
}

Then

const removeAnimal = (id) => {
  updateAnimalList(animalList.filter((x) => x.id != id));
};

Upvotes: 2

G&#246;khan
G&#246;khan

Reputation: 22

You can get the same function with useRef

const [animalList, updateAnimalList] = useState([]);

  const refRemoveFunction = useRef(() => {});

  const addAnimal = () => {
    updateAnimalList([
      ...animalList,
      {
        ...animals[Math.floor(Math.random() * animals.length)],
        onClick: refRemoveFunction
      }
    ]);
  };

  useEffect(() => {
    refRemoveFunction.current = (index) => {
      animalList.splice(index, 1);
      updateAnimalList([...animalList]);
    };
  }, [animalList]);

And you can use like this:

export function Animal(animal) {
  return (
    <div className="card">
      <h2>{animal.name}</h2>
      <button onClick={() => animal.onClick.current(animal.index)}>Remove</button>
    </div>
  );
}

I hope, It will be useful for you.

Upvotes: -1

Valentin
Valentin

Reputation: 11812

I would avoid storing onClick in animalList because removeAnimal captures a stale reference to a version of animalList.

updateAnimalList can be used like you did or it can also accept an updater function which receives the current value of animalList.

So by combining these two, I would end up with something like this:

const animals = [
  // the animals (should be defined outside of the component as it is not changing)
];

export default function App() {
  const [animalList, updateAnimalList] = useState([]);

  const addAnimal = () => {
    updateAnimalList([
      ...animalList,
      animals[Math.floor(Math.random() * animals.length)]
    ]);
  }

  const makeRemoveAnimal = (index) => () => {
    updateAnimalList((current) => [...current].splice(index, 1))
  };

  return (
    <div className="app">

        <button onClick={addAnimal}>addAnimal</button>
        {animalList.map((animalName, index) => {
          return (
            <Animal key={index} name={animalName} onClick={makeRemoveAnimal(index)} />
          )
        })}

    </div>
  );
};

Upvotes: 2

StepUp
StepUp

Reputation: 38199

It is not good to store link onClick in animalList because link becomes unactual.

It looks like you should send name of animal. When you will send name of animal, then it would be pretty simple to remove item of array:

// you need to send here name of animal  
const removeAnimal = () => {
    const updatedArray = animalList.filter( p=> p.name !== 'anaconda')
    updateAnimalList(prevAnimals => [...updatedArray]);
};  

Upvotes: -1

Related Questions