Why setState crashes browser

Question

I have two tabs that contains some bulletins and newses, respectively. And a badge on each tab to tell if all of the items has been viewed.

If all of bulletins or newses has been viewed, then the badge on that tab will hide, otherwise the badge will show up.

All needed calculations were defined in function checkBulletinHasNew and checkNewsesHasNew. But when I opened it up in browser, it crashed.

I'm pretty sure the causes of crash is the this.setState in these two function, because when I comment this.setState and replace it with console.log sentence, the browser works as usual.

How Can I fix it?

import React from 'react';
import {Tab, Tabs} from '../../../../../components/Tabs';
import {TitleBar} from '../../../../../components/TitleBar';
import List from './List.jsx'
import ListItem from './ListItem.jsx'

class MsgCenter extends React.Component {
  constructor() {
    super()
    this.checkBulletinHasNew = this.checkBulletinHasNew.bind(this)
    this.checkNewsesHasNew = this.checkNewsesHasNew.bind(this)
    this.state = {
      bulletinHasNew: false,
      newsesHasNew: false,
      active: true
    }
  }

  handleTabChanged() {

  }

  checkBulletinHasNew(bulletins) {
    if (bulletins && bulletins.length > 0) {
      for(var i = 0;i < bulletins.length;i++){
        if (!bulletins[i].viewed){
          this.setState({bulletinHasNew: true})
          //console.log('bulletings has un-viewed')
          return
        }
      }
      this.setState({bulletinHasNew: false})
      //console.log('bulletings are viewed')
      return
    }
  }

  checkNewsesHasNew(newses) {
    if (newses && newses.length > 0) {
      for(var i = 0;i < newses.length;i++) {
        if(!newses[i].viewed){
          this.setState({newsesHasNew: true})
          //console.log('newses has un-viewed')
          return
        }
      }
      this.setState({newsesHasNew: false})
      //console.log('newses are viewed')
      return
    }
  }

  componentWillUpdate(nextProps, nextState) {
    this.checkBulletinHasNew(nextProps.bulletins.items)
    this.checkNewsesHasNew(nextProps.newses.items)
  }

  componentDidMount(){
    this.checkBulletinHasNew(this.props.bulletins.items)
    this.checkNewsesHasNew(this.props.newses.items)
  }

  render() {
    return (
      <div>
        <TitleBar title="Message Center"></TitleBar>
        <Tabs showInkBar>
          <Tab label="Bulletins" value={0} badge={this.state.bulletinHasNew ?
            <span className="circleBadge">badge</span> :
            null
          }>
            <List>
              {
                this.props.bulletins.items.map(function (item) {
                  return (
                    <ListItem item={item} key={'bulletin.' + item.id}></ListItem>
                  )
                })
              }
            </List>
          </Tab>
          <Tab label="Newses" value={1} badge={this.state.newsesHasNew ?
            <span className="circleBadge">badge</span> :
            null
          }>
            <List>
              {
                this.props.newses.items.map(function (item) {
                  return (
                    <ListItem item={item} key={'news' + item.id}></ListItem>
                  )
                })
              }
            </List>
          </Tab>
        </Tabs>
      </div>
    )
  }
}

MsgCenter.defaultProps = {
  activeSubject: 'bulletins',
  bulletins: {
    isFetching: false,
    isRefreshing: false,
    page: 1,
    totalPage: 1,
    items: [
      {
        id: 1,
        title: 'This is bulletin 1',
        publicDate: 1461513600000,
        viewed: true
      },
      {
        id: 2,
        title: 'This is bulletin 2',
        publicDate: 1461427200000,
        viewed: true
      },
      {
        id: 3,
        title: 'This is bulletin 3',
        publicDate: 1461340800000,
        viewed: true
      },
      {
        id: 4,
        title: 'This is bulletin 4',
        publicDate: 1461254400000,
        viewed: true
      }
    ]
  },
  newses: {
    isFetching: false,
    isRefreshing: false,
    page: 1,
    totalPage: 1,
    items: [
      {
        id: 5,
        title: 'This is news 1',
        publicDate: 1458748800000,
        viewed: false
      },
      {
        id: 6,
        title: 'This is news 2',
        publicDate: 1458662400000,
        viewed: false
      },
      {
        id: 7,
        title: 'This is news 3',
        publicDate: 1458576000000,
        viewed: true
      },
      {
        id: 8,
        title: 'This is news 4',
        publicDate: 1458489600000,
        viewed: true
      },
    ]
  }
}

module.exports = MsgCenter

Answer 1

Thanks @Chris, that answer helps a lot, since I haven't read-through the docs carefully. That's true @Rajeesh Madambat, u can't call the setState method in those life cycle functions except for componentWillReceiveProps, however, that function doesn't be of any use to me here.

I'm new to reactjs, so when I want to control the appearance of a piece of component (in here is badge), all I come up with by reflex is to create a state to manage that. That's wrong because that state depends on props all the time.

Imagine this: I initial two states at the very beginning, but that's meaningless because the value of those two state depends on props.So I have to set those two state right after the component has received props. Which means there is no way to avoid that call-setstate-in-life-cycle trap.

If you have some value that depends on props all the time, don't make it as state(maybe props?), make it as computed value. And you compute those computed values in the render method. That problem happened to me just because I think in a wrong way in React.

So, with @wintvelt's great suggestion of code optimization, I can rewrite my code as below and it works as expected:

import React from 'react';
import {Tab, Tabs} from '../../../../../components/Tabs';
import {TitleBar} from '../../../../../components/TitleBar';
import List from './List.jsx'
import ListItem from './ListItem.jsx'

class MsgCenter extends React.Component {
  constructor() {
    super()
    this.checkItemsHasUnviewed = this.checkItemsHasUnviewed.bind(this)
    this.state = {
      bulletinHasNew: false,
      newsesHasNew: false,
      active: true
    }
  }

  checkItemsHasUnviewed(items) {
    let hasUnViewed = false
    if (items && items.length > 0) {
      for (let i = 0; i < items.length; i++) {
        if (!items[i].viewed) {
          hasUnViewed = true
          break
        }
      }
    }
    return hasUnViewed
  }

  render() {
    let bulletinHasUnviewed = this.checkItemsHasUnviewed(this.props.bulletins.items)
    let newsHasUnviewed = this.checkItemsHasUnviewed(this.props.newses.items)

    return (
      <div>
        <TitleBar title="Message Center"></TitleBar>
        <Tabs showInkBar>
          <Tab label="Bulletin" value={0} badge={bulletinHasUnviewed ?
            <span className="circleBadge"></span> :
            null
          }>
            <List>
              {
                this.props.bulletins.items.map(function (item) {
                  return (
                    <ListItem item={item} key={'bulletin.' + item.id}></ListItem>
                  )
                })
              }
            </List>
          </Tab>
          <Tab label="News" value={1} badge={newsHasUnviewed ?
            <span className="circleBadge"></span> :
            null
          }>
            <List>
              {
                this.props.newses.items.map(function (item) {
                  return (
                    <ListItem item={item} key={'news' + item.id}></ListItem>
                  )
                })
              }
            </List>
          </Tab>
        </Tabs>
      </div>
    )
  }
}

MsgCenter.defaultProps = {
  activeSubject: 'bulletins',
  bulletins: {
    isFetching: false,
    isRefreshing: false,
    page: 1,
    totalPage: 1,
    items: [
      {
        id: 1,
        title: 'This is bulletin 1',
        publicDate: 1461513600000,
        viewed: true
      },
      {
        id: 2,
        title: 'This is bulletin 2',
        publicDate: 1461427200000,
        viewed: true
      },
      {
        id: 3,
        title: 'This is bulletin 3',
        publicDate: 1461340800000,
        viewed: true
      },
      {
        id: 4,
        title: 'This is bulletin 4',
        publicDate: 1461254400000,
        viewed: false
      }
    ]
  },
  newses: {
    isFetching: false,
    isRefreshing: false,
    page: 1,
    totalPage: 1,
    items: [
      {
        id: 5,
        title: 'This is news 1',
        publicDate: 1458748800000,
        viewed: false
      },
      {
        id: 6,
        title: 'This is news 2',
        publicDate: 1458662400000,
        viewed: false
      },
      {
        id: 7,
        title: 'This is news 3',
        publicDate: 1458576000000,
        viewed: true
      },
      {
        id: 8,
        title: 'This is news 4',
        publicDate: 1458489600000,
        viewed: true
      },
    ]
  }
}

module.exports = MsgCenter

Feel free to comment on this.

Answer 2

I presume your constructor should be like this. Otherwise you are setting the state twice.

 constructor() {
        super()

        this.state = {
            bulletinHasNew: false,
            newsesHasNew: false,
            active: true
        }

This will result, the render method will render with default values that we set through the constructor. After that you can alter the values once the component mounted on the DOM.

You can remove componentWillUpdate and you can depend on componentDidMount to set the state. If you want to change setStatus once the component has been re-rendered use componentDidUpdate

Note: I would recommend to set both (bulletinHasNew,newsesHasNew) states together to avoid two render calls.

Hope this makes sense.

Answer 3

This is (probably) not an answer to the cause of your crash, but I suggest to change this:

checkBulletinHasNew(bulletins) {
  if (bulletins && bulletins.length > 0) {
    for(var i = 0;i < bulletins.length;i++){
      if (!bulletins[i].viewed){
        this.setState({bulletinHasNew: true})
        //console.log('bulletings has un-viewed')
        return
      }
    }
    this.setState({bulletinHasNew: false})
    //console.log('bulletings are viewed')
    return
  }
}

To this:

checkBulletinHasNew(bulletins) {
  if (bulletins && bulletins.length > 0) {
    var hasUnviewed = false;    
    for(var i = 0;i < bulletins.length;i++){
      if (!bulletins[i].viewed){
        hasUnviewed = true;
        //console.log('bulletins have un-viewed')
      }
    }
    this.setState({bulletinHasNew: hasUnviewed})
    //console.log('bulletins are viewed')
  }
}

Or even shorter, with an array.reduce():

checkBulletinHasNew(bulletins) {
  if (bulletins && bulletins.length > 0) {
    var hasUnviewed = bulletins.reduce(function(prevVal, curVal, i, array) {
      return prevVal || !curVal.a;
    },false);
    this.setState({bulletinHasNew: hasUnviewed})
    //console.log('bulletins are viewed')
  }
}

To get a cleaner function that is guaranteed only to do only 1 state-update.
Same for the other similar method.

Answer 4

The docs specifically state you shouldn't use set state in those lifecycle methods:

You cannot use this.setState() in this method. If you need to update state in response to a prop change, use componentWillReceiveProps instead.

I'm not sure exactly what it is doing, but I am guessing that setState triggers another "componentWillX" and in turn calls setState which triggers another "componentWillX" and in turn calls setState which triggers another "componentWillX" and in turn calls setState which triggers another "componentWillX" and in turn calls setState which ...

https://facebook.github.io/react/docs/component-specs.html