Wayne Ho
Wayne Ho

Reputation: 45

How to avoid using setState in shouldComponentUpdate() in React?

I created a React component for a forum view.

Source code can be seen: https://github.com/WayneHo25/YeePlus/blob/forum-feature/frontend/src/views/ForumPage/Sections/SectionPills.jsx

There are some tabs in the tops for several different forums.

When we click the tab, the state forumID is used to check which tab is clicked and which forum should be showed.

The state discussions is used to store the content of the forums.

The state isLoading is used to mark when the request is sending to the server.

import React from 'react'
// nodejs library that concatenates classes
import classNames from 'classnames'
// @material-ui/core components
import withStyles from '@material-ui/core/styles/withStyles'
import Tooltip from '@material-ui/core/Tooltip'
import Tab from '@material-ui/core/Tab'
import Tabs from '@material-ui/core/Tabs'
import CircularProgress from '@material-ui/core/CircularProgress'
// @material-ui/icons
import Reply from '@material-ui/icons/Reply'
// core components
import GridContainer from 'components/Grid/GridContainer.jsx'
import GridItem from 'components/Grid/GridItem.jsx'
import Button from 'components/CustomButtons/Button.jsx'
import Media from 'components/Media/Media.jsx'

import { getDiscussionsByForumID } from 'util/APIUtils'

import profile4 from 'assets/img/faces/card-profile4-square.jpg'

import sectionPillsStyle from 'assets/jss/material-kit-pro-react/views/blogPostsSections/sectionPillsStyle.jsx'

class SectionPills extends React.Component {
  constructor (props) {
    super(props)
    this.state = {
      discussions: [],
      isLoading: false,
      active: 0,
      forumID: 1
    }
    this.changeForum = this.changeForum.bind(this);
  }

  handleChange = (event, active) => {
    this.setState({ active });
  };

  changeForum(fid) {
    this.setState({ forumID: fid });
  }

  loadDiscussionList () {
    let promise

    promise = getDiscussionsByForumID(this.state.forumID)

    if (!promise) {
      return
    }

    this.setState({
      isLoading: true
    })

    promise
      .then(response => {
        const discussions = this.state.discussions.slice()

        this.setState({
          discussions: discussions.concat(response),
          isLoading: false
        })
      }).catch(error => {
        this.setState({
          isLoading: false
        })
      })
  }

  componentDidMount () {
    this.loadDiscussionList()
  }

  shouldComponentUpdate (nextState) {
    if (nextState.forumID != this.state.forumID) {
      this.loadDiscussionList()
    }
  }

  render () {
    const { classes } = this.props
    const flexContainerClasses = classNames({
      [classes.flexContainer]: true,
      [classes.horizontalDisplay]: false
    })
    const pillsClasses = classNames({
      [classes.pills]: true,
      [classes.horizontalPills]: false
    })
    const tabButtons = (
      <Tabs
        classes={{
          root: classes.root,
          fixed: classes.fixed,
          flexContainer: flexContainerClasses,
          indicator: classes.displayNone
        }}
        value={this.state.active}
        onChange={this.handleChange}
        centered={true}
      >
        <Tab
          key={1}
          label='YeePlus'
          classes={{
            root: pillsClasses,
            labelContainer: classes.labelContainer,
            label: classes.label,
            selected: classes.primary
          }}
          onClick={() => this.changeForum(1)}
        />
        <Tab
          key={2}
          label='Yeelight'
          classes={{
            root: pillsClasses,
            labelContainer: classes.labelContainer,
            label: classes.label,
            selected: classes.primary
          }}
          onClick={() => this.changeForum(2)}
        />
        <Tab
          key={3}
          label='Feedback'
          classes={{
            root: pillsClasses,
            labelContainer: classes.labelContainer,
            label: classes.label,
            selected: classes.primary
          }}
          onClick={() => this.changeForum(3)}
        />
      </Tabs>
    )
    const discussionList = []
    this.state.discussions.forEach((discussion, discussionIndex) => {
      discussionList.push(
        <Media
          key={discussion.id}
          avatar={profile4}
          title={
            <span>
              {discussion.title} <small>· 7 minutes ago</small>
            </span>
          }
          body={
            <p className={classes.color555}>
              {discussion.title}
            </p>
          }
          footer={
            <div>
              <Tooltip
                id='tooltip-tina'
                title='Reply to discussion'
                placement='top'
                classes={{ tooltip: classes.tooltip }}
              >
                <Button
                  color='primary'
                  simple
                  className={classes.footerButtons}
                >
                  <Reply className={classes.footerIcons} /> Reply
                </Button>
              </Tooltip>
            </div>
          }
        />)
    })
    const ColorCircularProgress = withStyles({
      root: {
        color: '#9c27b0'
      }
    })(CircularProgress)
    return (
      <div className={classes.section}>
        <GridContainer justify='center'>
          <GridItem xs={12} sm={10} md={8}>
            {tabButtons}
            <div className={classes.tabSpace} />
            {discussionList}
            <div className={classes.textCenter} >
            {
              this.state.isLoading
                ? <ColorCircularProgress /> : null
            }
            </div>
          </GridItem>
        </GridContainer>
      </div>
    )
  }
}

export default withStyles(sectionPillsStyle)(SectionPills)

error message:

Error: 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.

 50 |   return
  51 | }
  52 | 
> 53 | this.setState({
     | ^  54 |   isLoading: true
  55 | })
  56 | 

I want to keep the state isLoading, and I cannot find a solution to keep them both.

Upvotes: 1

Views: 476

Answers (1)

T.J. Crowder
T.J. Crowder

Reputation: 1074068

You can't set state in componentShouldUpdate. Instead, when issuing the setState for updating the forum ID, also start the process of loading discussions for that new forum. Since that's an async process, when it completes you need to check that the user hasn't clicked another forum in the meantime. Roughly:

constructor(props) {
  // ...
  this.loadId = 0;
  // ...
}

changeForum(fid) {
  this.setState({ forumID: fid, discussions: [] }); // *** Note clearing discussions!
  this.loadDiscussionList(fid);
}

loadDiscussionList(fid = this.state.forumID) {
  // *** To handle out-of-sequence completions and/or the user changing forums
  // while loading is happening, use a unique load identifier (more below)
  let loadId = ++this.loadId;
  let promise = getDiscussionsByForumID(fid);
  if (!promise) { // ??? Normally a method returning a promise always does so
    return;
  }

  this.setState({
    isLoading: true
  });

  promise
    .then(response => {
      const discussions = this.state.discussions.slice();

      this.setState(({forumID}) => {
        // Note the check, *within* the `setState` callback:
        // 1. We're still on the same forum, and
        // 2. There hasn't been a more recent call to `loadDiscussionList`
        if (forumID === fid && this.loadId === loadId) {
          return {
            discussions: discussions.concat(response),
            loading: false
          };
        }
      });
    })
    .catch(error => {
      // *** Usually you report an error
      // *** Same check as above
      this.setState(({forumID}) => {
        if (forumID === fid && this.loadId === loadId) {
          this.setState({isLoading: false});
        }
      });
    });
}

You'll also want to adjust (or remove) your shouldComponentUpdate. It needs to check at least forumID, isLoading, and discussions, possibly others.

Upvotes: 1

Related Questions