setState automatically calling another method in React

Question

I am experiencing strange behavior in my React App. When I add an expense to my store, the delete method deleteExpense automatically gets called, thus deleting the last item I added. This doesn't happen if I remove this.setState from my updateExpense method. Can't figure out why this is happening. Here is the code:

App.jsx:

import React from 'react';
import { observer } from 'mobx-react';
import DevTools from 'mobx-react-devtools';

import ExpenseStore from './stores/ExpenseStore';
import AddExpenseView from './components/AddExpenseView';
import CategoryView from './components/CategoryView';

var ExpenseStoreObj = new ExpenseStore();

@observer
export default class App extends React.Component {
  constructor() {
    super();
    this.state = {
      allExpenses: ExpenseStoreObj.expenses,
      expenseCategories: ExpenseStoreObj.expenseCategories
    }
    this.updateExpense = this.updateExpense.bind(this);
    this.deleteExpense = this.deleteExpense.bind(this);
    this.categorizeExpenses = this.categorizeExpenses.bind(this);
  }

  updateExpense(newExpense) {
    ExpenseStoreObj.addExpense(newExpense);
    this.setState({
      allExpenses: ExpenseStoreObj.expenses.slice()
    })
  }

  deleteExpense(ExpenseId) {
    ExpenseStoreObj.removeExpense(ExpenseId);
  }

  categorizeExpenses() {
    return ExpenseStoreObj.categorizeExpenses();
  }

  render() {
    return (
      <div className="app-container">
        <div className="col-xs-6 border">
          <AddExpenseView
            allExpenses={this.state.allExpenses}
            expenseCategories={this.state.expenseCategories}
            updateExpense={this.updateExpense}
            deleteExpense={this.deleteExpense}
          />
        </div>
        <div className="col-xs-6 border">
          <CategoryView
            categorizeExpenses={this.categorizeExpenses}
          />
        </div>
      </div>
    );
  }
}

ExpenseStore.js:

import { observable, computed, reaction } from 'mobx';

export default class ExpenseStore {
    constructor(props) {
        this.id = 1;
    }
    expenses = [];
    expenseCategories = ['Food', 'Clothing', 'Transport'];

    handleError(message) {
        alert(message);
    }

    addExpense(newExpense) {
        if (newExpense.expenseName == '') {
            this.handleError('Please enter expense name');
        }
        else if (newExpense.expenseCategory == '') {
            this.handleError('Please select expense categorry');
        }
        else if (newExpense.expensePrice == '') {
            this.handleError('Please enter expense value');
        }
        else {
            newExpense.id = this.id++;
            this.expenses.push(newExpense);
            this.categorizeExpenses();
            console.log('expense store: ', this.expenses);
        }
    }

    removeExpense(expenseId) {
        let expenseIndex = this.expenses.findIndex(x => x.id == expenseId);
        this.expenses.splice(expenseIndex, 1);
        console.log('after deleting expense', this.expenses);
    }

    categorizeExpenses() {
        let groupedResult = this.expenses.reduce(function (hash) {
            return function (r, a) {
                if (!hash[a.expenseCategory]) {
                    hash[a.expenseCategory] = { expenseCategory: a.expenseCategory, expensePrice: 0 };
                    r.push(hash[a.expenseCategory]);
                }
                console.log('hash[a.expenseCategory].expensePrice', hash[a.expenseCategory].expensePrice);
                console.log('a.expensePrice', a.expensePrice);
                hash[a.expenseCategory].expensePrice = parseInt(hash[a.expenseCategory].expensePrice) + parseInt(a.expensePrice);
                return r;
            };
        }(Object.create(null)), []);
        return groupedResult;
    }

}

On inspecting the call stack, I found that React is somehow calling handleExpenseDelete defined in ListExpenses.js (which gets the 'removeExpense' function of App.jsx as props.

ListExpenses.js

import React from 'react';
import PropTypes from 'prop-types';
import { observer } from 'mobx-react';

//@observer
export default class ListExpenses extends React.Component {
    constructor(props) {
        super(props);
        this.handleExpenseDelete = this.handleExpenseDelete.bind(this);
    }

    handleExpenseDelete(index) {
        this.props.deleteExpense(index);
    }

    render() {
        var listItems = this.props.allExpenses.map((obj, index) => {
            return (
                <tr key={index}>
                    <td>{obj.expenseName}</td>
                    <td>{obj.expenseCategory}</td>
                    <td>{obj.expensePrice}</td>
                    <td><button onClick={this.handleExpenseDelete(index)}>Delete</button></td>
                </tr>

            )
        });
        return (
            <div>
                <table className="table table-striped table-responsive">
                    <thead>
                        <tr>
                            <th>Item</th>
                            <th>Category</th>
                            <th>Price</th>
                            <th></th>
                        </tr>
                    </thead>
                    <tbody>
                        {
                            listItems
                        }
                    </tbody>
                </table>
            </div>
        );
    }
}

Answer 1

In your render method change the button onClick to something like below

<td><button onClick={() => this.handleExpenseDelete(index)}>Delete</button></td>

You are calling the method while rendering. The above code will call it only onClick.

EDIT:

When the below code is in place, the method(handleExpenseDelete) gets executed during render. Since {} braces indicates the expression inside it has to be executed, the expression inside is to call the function.

onClick={this.handleExpenseDelete(index)}

When you do it this way the expression inside {} is executed which results in a function and this function gets called when you click the button.

onClick={() => this.handleExpenseDelete(index)}

Answer 2

Reason is, onClick expect a function and you are returning a value by calling that function by onClick={this.handleExpenseDelete(index)}, its a function calling, handleExpenseDelete will get called on each rendering instead of on specific event.

You need to write it like this:

onClick={() => this.handleExpenseDelete(index)}

Or use .bind() and remove the binding from constructor, like this:

onClick={this.handleExpenseDelete.bind(this, index)}