Welcome to OGeek Q&A Community for programmer and developer-Open, Learning and Share
Welcome To Ask or Share your Answers For Others

Categories

0 votes
116 views
in Technique[技术] by (71.8m points)

javascript - Do I need to use setState(function) overload in this case?

Imagine such situation:

clickHandler(event) {
  var that = this;
  var cellIndex = event.target.id;

  if(that.inProcess) return;

  /* Temporarily clone state */
  var cloneState = JSON.parse(JSON.stringify(this.state));

  var currentCell = cloneState.gameCellArray[cellIndex];
  currentCell.shown = true;

  if (that.someCondition) {

          that.inProcess = true;
          setTimeout(function () {

              currentCell.shown = false;
              that.setState(cloneState);
              that.inProcess = false;

          }, 1000)

  }
  this.setState(cloneState);

}

Now I got worried in theory that where I clone state (fourth statement in clickHandler) it can happen that I don't get the most recent version of the state - since some setState calls from previous invocation of clickHandler are pending, is it valid assumption?

Now, my question is, if I rewrite above code in the following way (basically using setState with callback parameter), will it be safer? (in terms of not having out of synch state).

clickHandler(event) {
    var that = this;
    var cellIndex = event.target.id;
    if(that.inProcess) return;

    that.setState(function(prevState, props){

        var cloneState = JSON.parse(JSON.stringify(prevState));

        var currentCell = cloneState.gameCellArray[cellIndex];
        currentCell.shown = true;

        if (that.someCondition) {

                that.inProcess = true;
                setTimeout(function () {
                    currentCell.shown = false;
                    // Do I need to take some measures here also?
                    that.setState(cloneState);
                    that.inProcess = false;

                }, 1000)

        }
        return cloneState;

    })

}

Do I need to take some additional measure inside setTimeout also (the second rewritten version)?

ps. clickHandler is only function which changes state - and let's say gets called when user clicks some buttons.

pps. More generally what could go wrong (from synchronization point of view) with the state in either my first case or second version, elaborate answer would be appreciated, in order to better understand how to manage state in react.

See Question&Answers more detail:os

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
Welcome To Ask or Share your Answers For Others

1 Reply

0 votes
by (71.8m points)

This is a long answer. If you don’t want to read it all, scroll down to TLDR at the bottom.

Note that I describe some implementation details that could change in React 17+. This is why our docs are a little more vague so that most people don’t rely on implementation details too much. But in this case it seems like you’re specifically interested in how it really works so I have to be more specific than I’d like to be.

Now I got worried in theory that where I clone state (third statement in clickHandler) it can happen that I don't get the most recent version of the state - since some setState calls from previous invocation of clickHandler are pending, is it valid assumption?

No. At the time of this reply (React 16 and any earlier versions), this.state in the event handler is safe to read before you update the state yourself. So this code is fine:

handleClick() {
  var something = this.state.something;

It will give you the current state.

The only pitfall is that if you call setState yourself, you shouldn’t expect this.state to be updated immediately. So this code wouldn’t work:

handleClick(e) {
  this.setState({ something: e.target.value });
  var something = this.state.something; // Don't expect to get newValue here

Note: there is also another edge case pointed out in the comments: if you have several onClick handlers, the same pitfall applies: once you call setState() in a child event handler, you can’t rely on this.state being updated by the time the parent event handler runs. In fact it’s the reason this optimization is so useful: all setState() calls from a single browser event are batched, whether they happen in one or different components while the event bubbles.

Still, it isn’t a problem because if you called setState you already know what you set it to:

handleClick(e) {
  var newValue = e.target.value;
  this.setState({ something: newValue });
  // There's no need to "read" something from state.
  // Since you just set it, you already *know*
  // what you set it to. For example:
  doSomethingWith(newValue);

Now, there are cases when you want to update state based on the previous state. While you could have just read this.state in the event handler, this only works once:

handleIncrement() {
  // This will increment once:
  this.setState({ counter: this.state.counter + 1 });
  // These won't work because this.state.counter isn't updated yet:
  this.setState({ counter: this.state.counter + 1 });
  this.setState({ counter: this.state.counter + 1 });

To free you up from ever worrying about cases like this, React offers a different setState() overload that accepts a function. That function will receive the current state at the time the update is applied so you could safely use it. React will make sure to “thread” the current state through all the pending functions:

function increment(prevState) {
  return { counter: prevState.counter + 1 };
}

// ...
handleIncrement() {
  // Each function in the queue will receive the right state:
  this.setState(increment);
  this.setState(increment);
  this.setState(increment);
  // So this will increment three times.

As of React 16 and earlier, this overload is only useful when you call setState() multiple times from the same event handler. However, since it works in other cases too, we generally recommend using it any time your setState() call depends on the current state so that you don’t need to think about this at all. However if your code works without it, and attempts to rewrite it make it more confusing, don’t bother for now.

In the future we might also rely on it in more cases, but we’ll clearly call out any such changes in future releases. We will also be working on a more “natural” API for this since we’ve noticed people get confused by the contradiction because apparent imperative nature of setState(), and the more functional approach we recommend.


In your particular case, I actually think the first approach is simpler. You only call setState() once in your event handler (timeout happens later), so the pitfall about multiple consecutive calls doesn’t apply.

Your second approach that uses the functional setState() form actually doesn’t use it correctly, making the overall code more confusing. The functional form of setState() presumes that the function you pass to it is pure. For example, this is a pure function:

function increment(prevState) {
  return { counter: prevState.counter + 1 };
}

However, the function you pass not just calculates the next state, but also schedules a timeout, holds onto a piece of the state, mutates it in place, and inside the timeout calls setState again. This clearly is not how a pure function behaves. The rule of thumb is if you wouldn’t do something inside render(), you also shouldn’t do that inside the setState() updater function.

Again, in React 16 or less, rewriting your code to the functional form in this particular case wouldn’t be beneficial (I explained the reasons above: you’re just calling setState() once, and you’re not trying to read the state right after it). But if you do want to use the functional form, you need to make sure the function you pass is pure. The question is: where do you put the timeout logic, then?

My opinion is that the timeout logic would be better placed in the componentDidUpdate() lifecycle hook. This way it would be truly triggered by a state change—no matter where in the component it happened—as long as it satisfied the necessary conditions. For example, even if you had two buttons triggering the same state changes, they would both cause componentDidUpdate() to fire, and it could run the timeout logic depending on how the state changed.

Since your question was about implementing a memory game, based on this GitHub discussion, I wrote some pseudocode on how I would approach this task instead. Let me quote my answer here:

I think that if you split the timeout-related part of this logic into the componentDidUpdate lifecycle hook, the code could be easier to understand. There might also be a better way to model the state itself. The matching game seems like a “state machine” with a few different valid states (nothing selected, one item selected and waiting, two right items selected, two wrong items selected).

It might be worth encoding these possible game states more directly into your component state and think more carefully about how to represent them with objects. For example, it might be that instead of an array of cell values it is easier to think about an explicit state like:

{
  openedCells: [1, 2], // array of ids
  firstSelectedCell: 5, // could be null
  secondSelectedCell: 7, // could be null
}

and then implement conditional logic in componentDidUpdate, e.g.

handleClick(e) {
  // Are we waiting for a timeout? Reset it.
  if (this.resetTimeout) {
    clearTimeout(this.resetTimeout);
  }

  const id = ... // get it from target node, or bind event handler to ID in render()
  this.setState(prevState => {
    if (prevState.firstSelectedCell !== null && prevState.secondSelectedCell === null) {
      // There is just one selected cell. We clicked on the second one.
      return {
        secondSelectedCell: id
      };
    }
    // We are selecting the first cell
    // (either because we clicked to reset both or because none were selected).
    return {
      firstSelectedCell: id,
      secondSelectedCell: null
    };
}

componentDidUpdate(prevState) {
  if (prevState.secondSelectedCell !== this.state.secondSelectedCell) {
    // We just picked the second cell.
    if (isSamePicture(
      this.state.secondSelectedCell,
      this.state.firstSelectedCell
    ) {
      // Same picture! Keep them open.
      this.setState(prevState => {
         // Add them both to opened cells and reset.
         return {
           firstSelectedCell: null,
           secondSelectedCell: null,
           openedCells: [
             ...prevState.openedCells,
             prevState.firstSelectedCell,
             prevState.secondSelectedCell
           ]
         };
    } else {
      // Clear both in a second.
      this.resetTimeout = setTimeout(() => {
        this.setState({
          firstSelectedCell: null,
          secondSelectedCell: null,
        });
      }, 1000);
    }
}

Then, in the render method, you can show cells if either they are in openedCells or they are firstSelectedCell or secondSelectedCell.

I hope this helps! To sum up, here is the TLDR:

  • At least in React 16 (or earlier), reading this.state before the first setState() call in the event handler will give you the current state. But don’t expect it to be updated immediately after setState().
  • The functional overload of setState() protects against this pitfall, but it requires that the passed function is pure. Setting timeouts is not pure.
  • The componentDidUpdate() lifecycle hook might be a better place for setting timeouts that depend on the state.

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
OGeek|极客中国-欢迎来到极客的世界,一个免费开放的程序员编程交流平台!开放,进步,分享!让技术改变生活,让极客改变未来! Welcome to OGeek Q&A Community for programmer and developer-Open, Learning and Share
Click Here to Ask a Question

...