React error Maximum update depth exceeded

Question

I'm doing my first few experiments with React and in this component I am calling an external API to get a list of all NBA players, filter them by the teamId which was received as a component's prop and finally render the markup of the filtered players.

One consideration is that since I call the API and get a large list I keep it in the component's state so that new calls to this component would use that state instead of calling the API again. This is not production code and I don't own the API so I do this because I was getting a "Too many requests" message since I am continously trying stuff.

Anyway, when I try this code I get the already famous:

Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.

I've looked into the markup and I don't think I am making any method calls that would cause the render method to fire again and so forth, so I am at a loss as to why this is happening.

Thank you in advance for any help.

Here's the code in question:

 class Players extends Component {
    nbaPlayersUrl = "https://someUrl.com";
    state = {
        players: null,
        selectedTeamPlayers: null
    };

    render() {
        if (this.props.teamId === null) return null;

        if (this.state.players !== null) {
            var selectedTeamPlayers = this.filterPlayersByTeamId(this.state.players);
            var markup = this.getMarkup(selectedTeamPlayers);
            this.setState({selectedTeamPlayers: markup});
        } else {
            this.getPlayersList();
        }

        return (
            this.state.selectedTeamPlayers
        );
    }

    getPlayersList() {
        let api = new ExternalApi();
        let that = this;

        api.get(this.nbaPlayersUrl).then(r => {
            r.json().then(result => {
                let players = result.data.map(p => ({
                    id: p.id,
                    firstName: p.first_name,
                    lastName: p.last_name,
                    position: p.position,
                    heightInches: p.height_inches,
                    heightFeet: p.height_feet,
                    weightPounds: p.weight_pounds,
                    teamId: p.team.id
                }));

                that.setState({players: players});
                var selectedTeamPlayers = that.filterPlayersByTeamId(players);
                var markup = that.getMarkup(selectedTeamPlayers);
                that.setState({selectedTeamPlayers: markup});
            });
        });
    }

    filterPlayersByTeamId(players) {
        return players.filter(p => {
            return p.teamId === this.props.teamId;
        });
    }

    getMarkup(players) {
        var result = players.map(p => {
            <li key={p.id}>
                <div>
                    <label htmlFor="firstName">First Name</label> <input type="text" name="firstName" value={p.firstName} readOnly></input>
                </div>
                <div>
                    <label htmlFor="lastName">Last Name</label> <input type="text" name="lastName" value={p.lastName} readOnly></input>
                </div>
                <div>
                    <label htmlFor="position">Position</label> <input type="text" name="position" value={p.position} readOnly></input>
                </div>
            </li>
        });

        return (
            <ul>
                {result}
            </ul>
        );
    }
}

export default Players;

Shouldn't getPlayersList() has been written as an async callback? — Nima Bastani, Feb 11 '21 at 20:33

Jason Bellomy · Accepted Answer · 2021-02-12T02:08:09.647

@Sergio Romero - You CAN NOT set state in a render function, as that set state will call a new render, which will set state again and call a new render, and generates an infinite loop. Your loading of the data is in the render and setting state, which is also creating infinite loops. You need to completely re-write your render to only be a view of state and props (it should never manipulate or load data). I think what you want, is more like this:

class Players extends Component {
    nbaPlayersUrl = "https://someUrl.com";
    static propTypes = {
        teamId: PropTypes.number
    };
    static defaultProps = {
        teamId: null
    };
    constructor(props) {
        super(props);
        this.state = {
            players: null
        };
    }
    componentDidMount() {
        this.getPlayerList();
    }
    filterPlayersByTeamId(players, teamId) {
        return players.filter(p => {
            return p.teamId === teamId;
        });
    }
    getPlayersList = () => {
        let api = new ExternalApi();

        api.get(this.nbaPlayersUrl).then(r => {
            r.json().then(result => {
                let players = result.data.map(p => ({
                    id: p.id,
                    firstName: p.first_name,
                    lastName: p.last_name,
                    position: p.position,
                    heightInches: p.height_inches,
                    heightFeet: p.height_feet,
                    weightPounds: p.weight_pounds,
                    teamId: p.team.id
                }));

                this.setState({players});
            });
        });
    };
    render() {
        if (!this.props.teamId || !this.state.players) return null;

        const selectedTeamPlayers = this.filterPlayersByTeamId(this.state.players, this.props.teamId);

        return (
            <ul>
                {
                    selectedTeamPlayers.map(player => {
                        <li key={player.id}>
                            <div>
                                <label htmlFor="firstName">First Name</label><input type="text" name="firstName" value={player.firstName} readOnly></input>
                            </div>
                            <div>
                                <label htmlFor="lastName">Last Name</label><input type="text" name="lastName" value={player.lastName} readOnly></input>
                            </div>
                            <div>
                                <label htmlFor="position">Position</label><input type="text" name="position" value={player.position} readOnly></input>
                            </div>
                        </li>
                    })
                }
            </ul>
        );
    }
}

export default Players;

score 0 · Answer 2 · answered Feb 11 '21 at 20:35

if State and Props change the Component Will re-render. in your render() function:

 if (this.state.players !== null) {
      var selectedTeamPlayers = this.filterPlayersByTeamId(this.state.players);
      var markup = this.getMarkup(selectedTeamPlayers);
      // this.setState({selectedTeamPlayers: 
 }

Try changing commented line, every time players not null the Component state is updated therefore the component Render function will run again.

Nima Bastani · Answer 3 · 2021-02-11T22:30:49.657

As of the fact that we can not set state inside render function because it will cause side effect, you cannot call the getPlayersList() inside of the render function.

The solution mentioned by @Jason Bellomy is the proper way to solve this with calling getPlayerList inside of the componentDidMount, because it invoked immediately after a component is inserted into the tree, thus it's a place to set initial data for rendering a page.

class Players extends Component {
    nbaPlayersUrl = "https://someUrl.com";
    state = {
        players: null,
        selectedTeamPlayers: null,
    };
    
    componentDidMount(){
     this.getPlayersList();
    }

    render() {
        if (this.props.teamId === null) return null;

        if (this.state.players !== null && this.state.selectedTeamPlayers !== null) {
             return this.getMarkup(selectedTeamPlayers);
        } else {
           return (<span> Loading ... </span>);
        }
    }

    getPlayersList() {
        let api = new ExternalApi();
        let that = this;

        api.get(this.nbaPlayersUrl).then(r => {
            r.json().then(result => {
                let players = result.data.map(p => ({
                    id: p.id,
                    firstName: p.first_name,
                    lastName: p.last_name,
                    position: p.position,
                    heightInches: p.height_inches,
                    heightFeet: p.height_feet,
                    weightPounds: p.weight_pounds,
                    teamId: p.team.id
                }));
               
                 var selectedTeamPlayers = that.filterPlayersByTeamId(players);
             
                that.setState({
                      players: players, 
                      selectedTeamPlayers: selectedTeamPlayers, 
                });
            });
        });
    }

    filterPlayersByTeamId(players) {
        return players.filter(p => {
            return p.teamId === this.props.teamId;
        });
    }

    getMarkup(players) {
        var result = players.map(p => {
            <li key={p.id}>
                <div>
                    <label htmlFor="firstName">First Name</label> <input type="text" name="firstName" value={p.firstName} readOnly></input>
                </div>
                <div>
                    <label htmlFor="lastName">Last Name</label> <input type="text" name="lastName" value={p.lastName} readOnly></input>
                </div>
                <div>
                    <label htmlFor="position">Position</label> <input type="text" name="position" value={p.position} readOnly></input>
                </div>
            </li>
        });

        return (
            <ul>
                {result}
            </ul>
        );
    }
}

export default Players;

No. You are still setting state in the render, and that is a big NO NO. That leads to infinite loops. — Jason Bellomy, Feb 11 '21 at 21:14
Thanks for mentioning it, I have got confused, using the condition would reduce the number of re-renders to 1 but its already forbidden to use setState() inside render as you said. I have edit my answer too. — Nima Bastani, Feb 11 '21 at 21:24
you still call getPlayerList() in your render, which also sets state. even though it is not a direct setState call, the function executed still does. I have added an answer in which neither sets state nor calls a function that sets state in the render. Please refer to my answer above. — Jason Bellomy, Feb 11 '21 at 21:28
@niima - You are getting closer, however, now you are putting JSX in state. You should never put JSX in state https://stackoverflow.com/a/60418158/5419358 — Jason Bellomy, Feb 11 '21 at 22:03
Since React is a declarative language, the proper way is to tell the Render, I need you to render and show these players on the screen (your state data, which would be players). The render function should be like, okay. I have this list of players, and this is how I render them (that would be the JSX returned at the bottom). That follows the declarative pattern. By storing the JSX in state, you can now alter the JSX and alter it's state and tell the render how to render the data, which is more imperative programming than declarative. — Jason Bellomy, Feb 11 '21 at 22:11
Thanks for being such a start @Jason Bellomy, I've learned a lot. — Nima Bastani, Feb 11 '21 at 22:18

React error Maximum update depth exceeded

3 Answers3