Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: setState behaves sync when inside a promise callback and async when not. #20991

Closed
jasongornall opened this issue Mar 12, 2021 · 8 comments
Labels
Resolution: Expected Behavior Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@jasongornall
Copy link

jasongornall commented Mar 12, 2021

import ReactDOM from "react-dom";
import React, { useState } from "react";
import "./styles.css";

class Hello extends React.Component {
  constructor() {
    super();
    this.state = { data: [] };
  }

  async fetchJoke() {
    return Promise.resolve({
      id: Math.random(),
      joke: `${Math.random()}`
    });
  }

  // state is sync
  fetchAllJokesSync() {
    this.fetchJoke().then((joke) => {
      let jokes = [];
      console.log("before", this.state.data.length); // 0
      jokes.push(joke);
      console.log("before-2", this.state.data.length); //0
      this.setState({ data: jokes });
      console.log("before-3", this.state.data.length); // 1
      console.log("after", this.state.data.length); // 1
    });
  }
  // state is async
  fetchAllJokesASync() {
    const joke = {
      id: "wakka",
      joke: "wakka"
    };
    let jokes = [];
    console.log("before", this.state.data.length); //0
    jokes.push(joke);
    console.log("before-2", this.state.data.length); // 0
    this.setState({ data: jokes });
    console.log("before-3", this.state.data.length); // 0
    console.log("after", this.state.data.length); // 0
  }

  async componentDidMount() {
    this.fetchAllJokesSync();
    // this.fetchAllJokesASync();
  }

  render() {
    return (
      <div>
      </div>
    );
  }
}

const rootElement = document.getElementById("root");
ReactDOM.render(<Hello />, rootElement);

React version:
React 16.12.0
React-Dom 16.12.0

Steps To Reproduce

  1. Toggle between 2 functions in componentDidMount and observe changes to state and observe console

Link to code example:
https://codesandbox.io/s/badjokes-forked-3o08z?file=/src/index.js SetState

The current behavior

setState is sync inside a promise callback

The expected behavior

setState should always by async inside a promise callback

@jasongornall jasongornall added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Mar 12, 2021
@jasongornall
Copy link
Author

I recognize https://reactjs.org/docs/react-component.html#setstate This states that it does not always immediately update the component but when does it decide to do that and why? I'm worried this can lead to accidental bugs if this is non deterministic. Is there a way to force this to always be async for development purposes so people don't accidentally rely on this being sync?

@jasongornall
Copy link
Author

no but I'm running this on latest chrome.

@jasongornall
Copy link
Author

jasongornall commented Mar 12, 2021

I found a tweet from 2018 that suggests the right answer: https://twitter.com/dan_abramov/status/959507572951797761
Is this still the case?

@jordyvandomselaar
Copy link

jordyvandomselaar commented Mar 12, 2021

I'm not sure if it is still the case to be honest. I've since moved on to hooks which are a lot less weird to me =)

In any case, I strongly suggest not to depend on it being synchronous. Because setState() is more of a request than an order, React decides when it can safely update the state best. This is usually near to instant but could take a little bit longer if React thinks that's a good idea.

@markerikson
Copy link
Contributor

markerikson commented Mar 13, 2021

This is entirely standard and intentional behavior. Currently, React only batches state updates that happen within its own event handlers. Logic in a promise callback is outside that event handler tick and thus outside the batching wrapper, so it's processed synchronously

https://blog.isquaredsoftware.com/2020/05/blogged-answers-a-mostly-complete-guide-to-react-rendering-behavior/#render-batching-and-timing

You should also probably read Dan's extensive explanation at #11527 (comment)

Note that this is true regardless of whether you're using hooks or classes. Also, in Concurrent Mode, state updates are always batched all the time.

@eps1lon
Copy link
Collaborator

eps1lon commented Mar 13, 2021

Closing since this is expected behavior.

#20991 (comment) and are great answers #20991 (comment).

@jasongornall
Copy link
Author

These explanations are great, and thankyou for the links!! Would it be possible to get this officially documented?

@markerikson
Copy link
Contributor

FWIW, the React team is currently working on a complete rewrite of the docs:

reactjs/react.dev#3308

Not sure what specifically will be in there, but I'm hopeful that some of the behavior like this will be covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Expected Behavior Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

4 participants