(React) setState not updating

Hi friends,

setState is not updating values in the state for some reason when I pause the timer although it’s receiving correct parameters.

The only code you need to looks is updateState and pause functions.

Looked at it briefly. It could be because you’re not using a constructor/problems with “this”.

Usually in a stateful React component you want to initialize state like this:

class App extends React.Component {
   constructor(props) {
    super(props);

    this.state = {
       sessionLength: 10,
       restLength: 5,
       session: 10,
       rest: 5,
       isTicking: true,
       isSession: true,
       timeSwitch: false
  };
}

Also, this function is a mess:

 updateState = (
    session = this.state.session,
    rest = this.state.rest,
    isTicking = this.state.isTicking,
    isSession = this.state.isSession,
    timeSwitch = this.state.timeSwitch
  ) => {
    console.log(session, rest, isTicking, isSession, timeSwitch);
    this.setState(
      {
        session,
        rest,
        isTicking,
        isSession,
        timeSwitch
      },
      console.log(this.state)
    );
  };

You’re trying to assign those function arguments values within the parameters. Why?

And typically when you want to setState in a function you do something like this:

updateStateFunction = () => {
   this.setState({
      stateToUpdate: this.state.whateverIWantToUpdate,
      secondStateToUpdate: this.state.whateverElseIWantToUpdate
   })
}

I am using updateState as a callback function which I pass it down to a component level down which will be called with parameters to pass back some info.

import React from "react";
import Timer from "./Timer";

class App extends React.Component {
  constructor(props) {
    super(props);

    this.state = {
      sessionLength: 10,
      restLength: 5,
      session: 10,
      rest: 5,
      isTicking: true,
      isSession: true,
      timeSwitch: false
    };
  }

  updateState = () => {
    this.setState({
        session: this.state.session,
        rest: this.state.rest,
        // You can use boolean logic for the next three (i.e. when clicked/changed you can "not" it i.e. !this.state.isTicking)
        isTicking: this.state.isTicking, 
        isSession: this.state.isSession,
        timeSwitch: this.state.timeSwitch
      })
      console.log(this.state)
  };

  render() {

    return (
      <div className="App">
        <Timer
          sessionLength={this.state.sessionLength}
          restLength={this.state.restLength}
          session={this.state.session}
          rest={this.state.rest}
          isTicking={this.state.isTicking}
          isSession={this.state.isSession}
          timeSwitch={this.state.timeSwitch}
          updateState={this.state.updateState}
        />
      </div>
    );
  }
}

export default App;

Also, you should probably write your timer functions that are in Timer.js in your App.js and make Timer.js a stateless function i.e. you just pass the props to Timer.js and use it as a container component.

Thanks for trying to help but that’s not the problem here.

If you open up your console I am receiving all the props correctly from my pause function but setState is not setting the state even though all other functions in Timer component is using updateState function to set the state correctly.

Received props 8 5 false true why

I am receiving timeSwitch as “why” but after setting the state it becomes false.

Not having a constructor is going to cause problems. Check out the constructor documentation.

The constructor for a React component is called before it is mounted. When implementing the constructor for a React.Component subclass, you should call super(props) before any other statement. Otherwise, this.props will be undefined in the constructor, which can lead to bugs.

That’s probably part of your problem. Another issue seems to be that you’re trying to pass data back and forth between two components. React uses unidirectional data flow. Trying to pass data from parent to child and then from child to parent is going to complicate things greatly.

Another big issue is you’re repeatedly declaring the same let/const variables in your functions in Timer.js i.e. let { session, rest, updateState } = this.props; is declared in multiple functions.

This tutorial will help you a lot and is directly applicable to what you’re trying to make.

2 Likes

It’s really hard to pinpoint where your component “mess up” mainly for two factors:

1 - as mentioned above you declare a local variable to the Timer class with the same name as a prop

you have both this.session and session.
It’s super confusing understanding which is which.

2 - you call updateState so many time in sequence that is really hard to follow the state logic:
look at this

if (session === 0) {
  updateState(session, rest, true, true, true); // update session with some args
   this.stopSession(); // this calls updateState with different args
   this.resetSession(); // this calls updateState with different args
   this.startRest(); // this calls yet again updateState with different args
 }

Mind also that each updateState will update Timer as well since its reading those values as props…

My suggestion is to have only one component that handles all the state logic.
For example only App should have both the method that decrease the timer, as well as any logic that comes from user interaction (start/stop).

And let Timer only display what it receive from App.


p.s. @Nicknyr sorry to intrude, just a heads up.
Babel class fields proposal will let you write class property directly and will add a constructor` behind the scene.

babel repl calss fields to es6 classes

Despite hiding some crucial parts of an app behind a curtain i find the code more readable and concise :slight_smile:

1 Like

@Marmiz @Nicknyr Thanks for the tips guys! I will give it a try per your suggestions :).

BTW, do you think introducing redux will help better with state management here?

@Marmiz Also I am concerned about this approach.

For example only App should have both the method that decrease the timer, as well as any logic that comes from user interaction (start/stop).

Because I am afraid this will put too much code in my App.js therefore making my “root” component hard to read.

setState is asynchronous.
When you call updateState in pause you call it with “why” at the last parameter and then you immediately call updateState in stopSession with old parameters. After both updateState executes (asynchronously!!!, when React finds it convenient) you get the result - first updateState with “why” and after that second updateState resets everything back.

Possible solution:

  pause = () => {
    const { session, rest, isSession, updateState } = this.props;
    // updateState(session, rest, false, isSession, "why");

    if (isSession) {
      clearInterval(this.sessionInterval); // move it here from stopSession
      // return this.stopSession();
      updateState(session, rest, false, isSession, "why");
    }
    return this.stopRest(); // do the same here
  };
1 Like

Not for you in this case, no. You’re already basically trying to do what Redux does. Keep fighting through this and try to nail doing it with state/props passing. As other have said, you need to sort out the naming. You also need to look at reducing duplication.

Keep going with just state/props in React for as long as you can. The better you get at that, the quicker you’ll understand why Redux or MobX or whatever (or looking forwards, just Context and hooks in React itself, hopefully later this year) might be something useful to introduce to an app (or not!)

1 Like

@jenovs @DanCouper

Really appreciate your feedbacks. I ended up refactoring my code so that I am managing my state from App.js instead of Timer.js and it made my life so much easier.

Thanks!

1 Like