Nested asynchronous setState and setState inside Promises

[mikekasprzak]

I've been upgrading my codebase from Preact 8 to 10. In the Preact X upgrading guide it says not to use state synchronously anymore, which is fine, but I want to be sure I'm not potentially falling into a race condition or something.

I'll start with a simple example.

I have a component acting as a button. The $Love code makes an HTTP fetch and returns a promise. The Preact 8 code looked something like this.

onClick() {
  if (this.state.loved) {
    $Love.Remove(this.props.node.id)
    .then(() => {
      this.setState({'loved': false});
    });
  }
  else {
    $Love.Add(this.props.node.id)
    .then(() => {
      this.setState('loved': true});
    });
  }
}

I'm aware that if the code wasn't asynchronous, the solution would be something like this.

onClick() {
  this.setState(prevState => {
    if (prevState.loved) {
      Love.Remove(this.props.node.id);
      return {'loved': false};
    }
    else {
      Love.Add(this.props.node.id);
      return {'loved': true};
    }
  });
}

Unfortunately it is, so should I just wrap the original code in a throwaway setState callback that returns null? In essence use the setState callback to ensure my logic is based on the latest state.

onClick() {
  this.setState(prevState => {
    if (prevState.loved) {
      $Love.Remove(this.props.node.id)
      .then(() => {
        this.setState({'loved': false});
      });
    }
    else {
      $Love.Add(this.props.node.id)
      .then(() => {
        this.setState('loved': true});
      });
    }
    return null;
  });
}

Tangentially, I was running into issues with more creatively nested setState's. I worked around the problem by further rewriting my code to only make a single setState (and not reading state until writing via the callback), but I'd like to know if the following are even legal ways to use setState.

// Asynchronous
onClick1() {
  this.setState(prevState => {
    if (prevState.loved) {
      $Love.Remove(this.props.node.id)
      .then(() => {
        this.setState({'loved': false});
      });
    }
    else {
      $Love.Add(this.props.node.id)
      .then(() => {
        this.setState('loved': true});
      });
    }
    return {'clicks': prevState.clicks+1};
  });
}

// Synchronous
onClick2() {
  this.setState(prevState => {
    // Yes I realize I could return {'loved': !prevState.loved, 'clicks': prevState.clicks+1}
    this.setState({'loved': !prevState.loved});
    return {'clicks': prevState.clicks+1};
  });
}

Am I at risk of corrupting anything if a setState happens within a setState callback?

Thanks!

[marvinhagemeister]

I have a component acting as a button. The $Love code makes an HTTP fetch and returns a promise. The Preact 8 code looked something like this: (...)

To be honest the code looks perfectly fine and I'd keep the same for Preact 10. The text in the upgrade guide refers to the fact that you cannot immediately read the new values from this.state. In both Preact 8.x and 10.x the update is applied asynchronously to the DOM.

state = { foo: false }

// Preact 8.x
this.setState({ foo: true });
console.log(this.state.foo) // Logs: true

// Preact 10.x
this.setState({ foo: true });
console.log(this.state.foo) // Still logs previous value: false

That's what that section is referring to.

The setState on its own request an update but doesn't really have any concept of async itself. The scheduling semantics of Preact's renderer have changed slightly with 10.x compared to 8.x, but setState is treated the same.

The easiest way to avoid a race condition here is to disable the button for as long as your async code runs:

onClick() {
  // Button is disabled based on this state
  this.setState({ disabled: true })

  const loved = this.state.loved;
  // We don't care which action we fire, just that it returns a promise where
  // we need to attach a `.finally()` to reset disabled state.
  const promise = loved ? $Love.Remove(this.props.node.id) : $Love.Add(this.props.node.id)

  promise
    .then(() => {
      // Both actions seem to invert the "loved" flag in the original example
      this.setState({ loved: !loved });
    })
    // Make sure to re-enable button again, regardless if the operation
    // was successful or threw an error
    .finally(() => this.setState({ disabled: false });
}

If you want to get more fancy and actually abort the request, then I'd recommend looking into AbortController. It's the web's native way to cancel anything async. Basically you'd store an AbortSignal in your state so that the next click sees that an action is already in process and can cancel it.

Am I at risk of corrupting anything if a setState happens within a setState callback?

I'm not even sure if we have a test for that. This would certainly fall under the category of "undefined behavior".

[mikekasprzak]

To be honest the code looks perfectly fine and I'd keep the same for Preact 10. The text in the upgrade guide refers to the fact that you cannot immediately read the new values from this.state. In both Preact 8.x and 10.x the update is applied asynchronously to the DOM.

Okay, I think this is where I was lead astray. I misinterpreted what I read as if this.state was somehow unsafe to use now. Also when I read "asynchronous", my imagination got away with me, and probably assumed threads were being spawned or something inconvenient. I'm guessing what actually happens now is setState queues the changes, then applies the changes and runs the callbacks at some later time, instead of doing them right away. 😅

Am I at risk of corrupting anything if a setState happens within a setState callback?

I'm not even sure if we have a test for that. This would certainly fall under the category of "undefined behavior".

Got it. In hindsight I probably don't need this, but FWIW I don't think it works. 😆

Thanks so much for your help!

[marvinhagemeister]

Exactly, in both Preact 8.x and Preact 10.x setState is more of a request to update and the update is queued. But in Preact 8.x we did mirror the new state values immediately on this.state.