manas
manas

Reputation: 6400

React list rendering wrong data after deleting item

I have a simple list of student objects with name and their scores in state.

Their name is bound to <b>{student.name}</b> and their score is bound to

<input type="text" defaultValue={student.score}/>

Whenever I want to delete the first student from this list and re-rendering the component by calling set state.

The input tag of the second student showing the score of the first student instead of having its own. Why this happening where I am doing it wrong ??

Here is my code jsbin

class App extends React.Component{
  constructor(){
    super();
    this.state ={
      students:[{name:"A",score:10},{name:"B",score:20},{name:"C",score:30}]
    }
  }
  
  onDelete(index){
    this.state.students.splice(index,1);
    this.setState(this.state);
  }
  
   render(){
     return(
       <div>
         {this.state.students.map((student,index)=>{
              return(
                <div key={index}>
                    <b>{student.name}</b> - <input type="text" defaultValue={student.score}/>
                     <button onClick={this.onDelete.bind(this,index)}>delete</button>
                </div>                
              )
         })}
       </div>
     )
   }
}


ReactDOM.render(<App/>,document.getElementById("main"));

Upvotes: 17

Views: 9359

Answers (3)

Nguyễn Văn Phong
Nguyễn Văn Phong

Reputation: 14218

Best practice

To be honest, you shouldn't mutate state data directly. You should clone then do your task like this

onDelete(index) {
    const newStudents = [...this.state.students];
    newStudents.splice(index, 1);
    this.setState({ students: newStudents });
}

Why can't I directly modify a component's state, really?

The previous state will be polluted with your mutation. Due to which, the shallow compare and merge of two states will be disturbed or won't happen, because you'll have only one state now. This will disrupt all the React's Lifecycle Methods.


Your problem & solution

  1. In case that you use the input readOnly, you should change from defaultValue to value then your code work well.
<input type="text" value={student.score} readOnly />

class App extends React.Component {
  constructor() {
    super();
    this.state = {
      students: [
        { name: "A", score: 10 },
        { name: "B", score: 20 },
        { name: "C", score: 30 }
      ]
    };
  }

  onDelete(index) {
    const newStudents = [...this.state.students];
    newStudents.splice(index, 1);
    this.setState({ students: newStudents });
  }

  render() {
    return (
      <div>
        {this.state.students.map((student, index) => {
          return (
            <div key={index}>
              <b>{student.name}</b> -{" "}
              <input type="text" value={student.score} readOnly />
              <button onClick={this.onDelete.bind(this, index)}>delete</button>
            </div>
          );
        })}
      </div>
    );
  }
}

ReactDOM.render(<App/>, document.getElementById('app'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/17.0.1/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/17.0.1/umd/react-dom.production.min.js"></script>

<div id="app"></div>

  1. Otherwise, you should provide the unique key. If student.name is not unique, you can randomly GUID like this.
const getGUID = () => "id" + Math.random().toString(16).slice(2);
>  <div key={getGUID()}>

const getGUID = () => "id" + Math.random().toString(16).slice(2);

class App extends React.Component {
  constructor() {
    super();
    this.state = {
      students: [
        { name: "A", score: 10 },
        { name: "B", score: 20 },
        { name: "C", score: 30 }
      ]
    };
  }

  onDelete(index) {
    const newStudents = [...this.state.students];
    newStudents.splice(index, 1);
    this.setState({ students: newStudents });
  }

  render() {
    return (
      <div>
        {this.state.students.map((student, index) => {
          return (
            <div key={getGUID()}>
              <b>{student.name}</b> -{" "}
              <input type="text" defaultValue={student.score} />
              <button onClick={this.onDelete.bind(this, index)}>delete</button>
            </div>
          );
        })}
      </div>
    );
  }
}

ReactDOM.render(<App/>, document.getElementById('app'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/17.0.1/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/17.0.1/umd/react-dom.production.min.js"></script>

<div id="app"></div>

However, in this case, Unstable keys will cause harmful performance because component instances - Virtual DOM & DOM node - actual DOM will always unnecessary recreated.

In short, depends on your data and behavior, you can choose the correct way to complete it.

Upvotes: 0

Aswin K
Aswin K

Reputation: 343

It is always better to use unique id as a key instead of index. If your JSON does not provide a unique Id, you can use something like uuid npm module which generates it for you. This is the link https://www.npmjs.com/package/uuid.

You can then import it and use as below

import { v4 } from 'uuid'; //there are 5 versions. You can use any.

then use it to generate uique Id by calling the v4 function like below

id={v4()}

Upvotes: 3

BradByte
BradByte

Reputation: 11093

It's because you're using key={index} instead of a value unique to the student.

When the array is modified the students after the index removed will have the wrong key and React will not register a key change to rerender with the updated data.

You should instead use something like this...

<div key={student.name}>

assuming that student.name is unique.

Upvotes: 43

Related Questions