Missing state value after function binding in React

Question

Im new to React and have some trouble right now.
I have component, which catches some object as props and few functions to change state once at few seconds:

export default class Header extends React.Component {
  constructor(props) {
    super(props)
    this.state = {
      variants:   props.variants,
      background: props.variants[0].background
    }
  }
  setTimer () {
    const { variants } = this.state
    clearTimeout(this.timeout)
    this.timeout = setTimeout(this.updateBackground.bind(this), 1000)
  }
  updateBackground () {
    console.log(`Keys ${this.state.variants}`);
    const { variants }    = this.state
    const { background }  = variants[parseInt(Math.random() * 5)]
    setState({
      background: background
    }, this.setTimer)
  }
  componentDidMount() {
    this.setTimer()
  }
  render() {
    const { background } = this.state
    return (
            <div className="header-image"> <img src={ background } /> </div>
      )
  }
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.min.js"></script>

My problem is: after

this.updateBackground.bind(this)

call, updateBackground lost all state values, e.g.

this.state.variants

is defined, but its no more contains objects, e.g

this.state.variants[0] === undefined

Explain to me please what i doing wrong :)

@AndrewLi, problem is earlier, than this.setState, this.state.variants[0].background is undefined — Nikita Goncharov
– Nikita Goncharov, Commented Jun 1, 2017 at 12:51
It doesn't looks like you need to store variants in state, you can just use the value from props directly. You should avoid putting props into initial state, and if you do, prefix the props with 'initial' - like "initialVariants": see medium.com/@justintulk/… — Tom Davies
– Tom Davies, Commented Jun 1, 2017 at 12:52
@NikitaGoncharov - can you add in the code that calls your component to the question? I'm expecting something like <Header variants={['abc.jpg', ...]} />? That should help people rule out the props not being passed in correctly. — Tom Davies
– Tom Davies, Commented Jun 1, 2017 at 13:01

vijayst · Accepted Answer · 2017-06-01 12:59:32Z

1

I am guessing a little bit here. The variants in props maybe empty initially. If the props are going to change, set state in componentWillReceiveProps

componentWillReceiveProps(nextProps) {
  if (nextProps.variants !== this.props.variants) {
    this.setState({ variants: nextProps.variants });
  }
}

The other option is what Tom Davies suggested. Use the props directly.

  updateBackground () {
    const { variants }    = this.props
    const { background }  = variants[parseInt(Math.random() * 5)]
    this.setState({
      background
    }, this.setTimer);
  }

answered Jun 1, 2017 at 12:59

vijayst

22.1k18 gold badges73 silver badges113 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Nikita Goncharov Over a year ago

The problem is what component recieved props, i check this in setTimer function, but when i call from setTimer function updateBackground, it dont have props values.

Ematipico · Accepted Answer · 2017-06-01 13:03:15Z

0

The problem is this line of code

this.timeout = setTimeout(this.updateBackground.bind(this), 1000)

When you call setTimeout, the very moment you bind it, you lost the scope of the class. Try this

  constructor(props) {
    super(props)
    this.state = {
      variants:   props.variants,
      background: props.variants[0].background
    }
    this.updateBackground = this.updateBackground.bind(this)
  }

And

this.timeout = setTimeout(() => this.updateBackground(), 1000)

answered Jun 1, 2017 at 13:03

Ematipico

1,2441 gold badge10 silver badges14 bronze badges

2 Comments

Nikita Goncharov Over a year ago

Unfortunately, this solution dont help me ;(

Ematipico Over a year ago

Even if you try to pass props and state as parameter to your function? setTimeout(() => this.updateBackground(state, props), 1000). Of course you have to extract them

Tom Davies · Accepted Answer · 2017-06-02 08:15:55Z

0

You can make quite a few changes to improve the readability and maintainability of your component. I've assumed that it should update every 1000ms and so swapped to use setInterval instead to avoid having to keep resetting the timer - since updating the image is not a long running operation anyway.

Additionally, I've added handling for when the component unmounts to stop the interval/timer from continuing to try and run and act on a component that is no longer there.

Component

export default class Header extends React.Component {

    constructor(props) {
        super(props);

        // We can just use props, don't need to copy variants
        // into state since it's never changed.
        this.state = {
            currentBackground: props.variants[0].background,
            intervalId: null
        };

        // I'll bind everything in the constructor, so it's
        // only done once and removes clutter from methods.
        this.updateBackground = this.updateBackground.bind(this);
    }

    componentDidMount() {
        // Do everything we need to on startup here, since it's only
        // setting the update interval, won't break it out into
        // separate functions.
        this.setState({
            intervalId: setInterval(() => this.updateBackground(), 1000)
        });
    }

    componentWillUnmount() {
        // When the component is unmounted, stop the interval.
        clearInterval(this.state.intervalId);
    }

    updateBackground() {
        // Assuming you wanted a whole number here, in which case
        // floor() makes more sense than parseInt(). We should use
        // variants.length rather than assuming there are 5 entries.
        let index = Math.floor(Math.random() * this.props.variants.length);

        this.setState({
            currentBackground: this.props.variants[index].background
        })
    }

    render() {
        // Somewhat opinionated, but there doesn't seem to be any reason
        // to wrap this in a <div>. You could then rename this component
        // and reuse it anywhere you wanted a random image.
        return <img className="header" src={this.state.currentBackground} />;
    }
}

Header.propTypes = {
    variants: React.PropTypes.array.isRequired
};

Called like...

<Header variants={[{ background: 'abc.png' }, { background: 'def.png' }]} />

edited Jun 2, 2017 at 8:15

answered Jun 1, 2017 at 13:17

Tom Davies

8997 silver badges20 bronze badges

2 Comments

Nikita Goncharov Over a year ago

I am use your code and get same error "Uncaught TypeError: Cannot read property 'background' of undefined"

Tom Davies Over a year ago

@NikitaGoncharov, there was a typo in the propTypes declaration (fixed now if you want to try again) - that wouldn't have caused the issue you described though. It sounds like maybe you're passing an array of strings in, rather than objects? What does render() look like in the component you're using Header in?

Collectives™ on Stack Overflow

Missing state value after function binding in React

3 Answers 3

1 Comment

2 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related