React: Make setState return a promise

Created on 2 Dec 2014  路  48Comments  路  Source: facebook/react

_setState()_ currently accepts an optional second argument for callback and returns _undefined_.

This results in a callback hell for a very stateful component. Having it return a promise would make it much more managable.

It is somewhat of a convention in JS world to have API symmetric - if a method accepts a callback, it returns a promise. if it returns a promise, it can accept a callback.

In my case, I have an editable table. If user presses enter, he is moved to the next row (call setState). If the user is on a last row, I'd like to create a new row (call setState) and focus on this new row (call setState). Currently the only way is to achieve this is to have a nested callback hell.

Core Utilities Feature Request

Most helpful comment

Using componentDidUpdate aside, Promises are a logical way of handling the post setState hook. And promises reflect the current state-of-opinion in the javascript community.

this.setState({
  selected: input
}).then(function() {
  this.props.didSelect(this.state.selected);
}.bind(this));

Is common React code, and much more readable than a optional callback argument.

Bump.

All 48 comments

Do you have any ideas about what to do with React.render which also accepts a callback but also returns a value?

There are a few other techniques that can be used. You can have the row's componentDidMount call focus so that it's auto-focused when it is created.

The new style refs are callbacks that get fired when the child is mounted. That can be used to trigger a focus without the need of wrapping it.

You can also use the componentDidUpdate life-cycle hook as a component gets updated to ensure that the focused thing (according to your business logic) always retains focus (e.g. the newest one).

I think that these are probably better alternatives that don't rely on the imperative nature of the source of your state change. E.g. you're tying it to the action that triggered the event. What if you add another state transition to the same state? Isn't it confusing that you can end up in the same state with different side-effects? That's part of the beauty of React, that you can avoid that.

Do you think that one of those patterns could replace your callback?

Even if it isn't... Honestly, the current batching strategy comes with a set of problems right now. I'm hesitant to expand on it's API before we're sure that we're going to keep the current model. I think of it as a temporary escape until we figure out something better.

Does that seem fair?

In my experience, whenever I'm tempted to use setState callback, I can achieve the same by overriding componentDidUpdate (and/or componentDidMount).

I was wondering the same. Thanks @gaearon for the trick it works as expected.

Using componentDidUpdate aside, Promises are a logical way of handling the post setState hook. And promises reflect the current state-of-opinion in the javascript community.

this.setState({
  selected: input
}).then(function() {
  this.props.didSelect(this.state.selected);
}.bind(this));

Is common React code, and much more readable than a optional callback argument.

Bump.

Why would you use that pattern over this?

this.setState({
  selected: input
});
this.props.didSelect(input);

It is in fact probably an anti-pattern to fire an event after updates in that case because it may triggers multiple rendering passes and multiple layout passes which has significantly worse performance implications.

The two code examples are functionally different. Anytime the parent need the child's updated DOM in place, or pending states resolved would be a candidate for the promise.

To come up with a contrived example, lets say the selection state changes the rendered component's width, and the parent needs to calculate a new position to account for this new child width.

Yes these DOM issues can be accounted for with componentDidUpdate, or others. But that's not the point. The point of this issue is to transition the setState interface to something more inline with JS community standards.

The problem with existing JS community practices is that they can often make code difficult to reason about. The nice thing about lifecycle methods is that they make it easy to reason about. There aren't side effects happening in other unexpected places due to some callback. If everything can be handled through lifecycle methods then I would personally advocate removing the setState() callback altogether.

@zzz6519003 It's a callback that can be passed as the second argument to setState() to be called when it is done. See https://facebook.github.io/react/docs/component-api.html#setstate

Do you have any ideas about what to do with React.render which also accepts a callback but also returns a value?

Two solutions, which aren't mutually exclusive:
(a) the callback gets the component as an argument / the promise returns the component. This will work for all cases where the component value is not needed synchronously.
(b) similarly to how ref={...} takes a function, the caller can supply a 2nd callback that gets called with the component instance, right after ReactDOM.render is done. Though in that case, one would wonder why not simply require that the consumer of ReactDOM.render add a ref={...} prop to the element themselves. In other words:

let el = ReactDOM.render(<Foobar />, div);
doSomethingWith(el);

becomes

let el;
ReactDOM.render(<Foobar ref={ref => el = ref} />, div);
doSomethingWith(el);

This is assuming that ref callbacks get called synchronously here.

setStateMixin using bluebird. It creates . this.setStateAsync for the current context,
I wanted to run promisifyAll on the React prototype but it seems that React.createClass hides its prototypes which is sad.

import Promise from "bluebird";

export default {
  componentWillMount() {
    this.setStateAsync = Promise.promisify(this.setState);
  },
};
import React from "react";
import Promise from "bluebird";

// ES6 class - have not tested this example (wrote it in the comment here) but should work
export default class Test extends React.Component {
  constructor() {
    super();
    this.setStateAsync = Promise.promisify(this.setState);
  }
}

so far this has worked fine in the situation of e.g.

{
  handleDoSomething() {
    return this.setStateAsync({
      loading: true,
    }).then(this.loadSomething).then((result) => {
      return this.setStateAsync({result, loading: false});
    });
  }
}

This is really dirty, it will promisify the current context functions that includes reacts and your own. only use it when your lazy and not in production.

import Promise from "bluebird";

export default {
  componentWillMount() {
    Promise.promisifyAll(this);
  },
};

@philikon

let el = ReactDOM.render(<Foobar />, div);
doSomethingWith(el);

becomes

let el;
ReactDOM.render(<Foobar ref={ref => el = ref} />, div);
doSomethingWith(el);

Wouldn't it be correct if it becomes

ReactDOM.render(
    <Foobar />
).then((foobarRef) => {
    doSomethingWith(foobarRef);
}).catch((error) => {
    logCatchedRenderingError(error);
});

This way I have the opportunity to

  • work with an ref as soon as the element was rendered
  • catch rendering errors on specific components ... currently the whole app explodes if one component throws an error during rendering ...
  • use promises for other functions (setState) as well

:+1: getting promises for state based changes would be a big plus for shallow testing component lifecycles

Would this be syntactically sound?: Promise.resolve( this.setState({ ... }) );

Using the bluebird Promise library.

+1 for this. since there is the possibility of a callback then it should be working with a promise. If it's an antipattern then remove the callback too.

:+ IF it doesn't add too much bloat.

How about this?

class AbstractComponent extends React.Component {
    setStatePromise(newState) {
        return new Promise((resolve) => {
            this.setState(newState, () => {
                resolve();
            });
        });
    }
}

It's very useful to have an option to use promises instead of callbacks.

@spectrox Or you can avoid introducing a new base class entirely:

function setStatePromise(that, newState) {
    return new Promise((resolve) => {
        that.setState(newState, () => {
            resolve();
        });
    });
}

I already had one, so, it was the simplest solution to add such function in it. And it's hard to add helper function (or mixin) in an ES6 environment, where every js file works in it's own scope, you'll have to import it everytime.
So, it depends on an environment which option to choose 馃憤

+1 , I'd love the promise way.

Guys support es5, so i don't think they will add it before moving to es6. syranide's solution is well.

PR #8282 related to this issue.

One thing you can do with Promises that you can't as easily do with callbacks is return them.

function fooHappens(foo) {
    return this.setState({foo: foo});
}

function barHappens(bar) {
    return this.fooHappens(bar.foo).then(function() {
        console.log(bar);
    });
}

If you'd happen to want to extend setState inside your ES6 component definition to be a thenable, you could do that (expanding on @spectrox and @syranide proposals ):

  setState(state, callback = () => {}){
    return new Promise((resolve, reject) => {
      super.setState(state, () => {
        if (typeof callback != "function") { reject(`${callback} is not a function`)}
        else {resolve(callback())}
      })
    })
  }

Though I see how this could be considered an antipattern, i feel like it could be useful in some cases when you need to do something with a new state that does not involve rerendering the component.

In my case it's usually involves changing the canvas context (based on the new rendered dimensions for example) and managing it separately from react update cycle(since react really shouldn't care of what's happening inside the canvas). But the cases probably would differ.

I believe setting state becomes asynchronous with implementation of React Fiber?

Not yet, but it will be async by default eventually.
However it is already async now in some cases.

@ConAntonakos check out this section in the documentation: State Updates May Be Asynchronous

What are the specific unknowns about returning a promise from setState? Just thinking:

  1. Committing to Promises. Does this rule out possible features?
  2. What actually triggers resolve/reject? Flushing of the update queue, or the individual setState transaction?
  3. Are there performance issues with producing a promise for every setState call that might otherwise never get used?

For the last item: what if setState returned an object that implemented the Promise public API? It wouldn't create a promise for every setState call, only if .then() was invoked.

or, what if React.Component implemented .then, resolving whenever the component's state transaction queue flushed?

Committing to Promises. Does this rule out possible features?

Synchronous resolve would be one. setState would still be synchronous in some cases.

Are there performance issues with producing a promise for every setState call that might otherwise never get used?

Yes. We need to "remember" to call that callback at the right point in time. So we have to keep a list of them for components that provided the callback. However with Promises we'd have to always do that for every single update just in case somebody happens to use the promise.

For the last item: what if setState returned an object that implemented the Promise public API? It wouldn't create a promise for every setState call, only if .then() was invoked.

Seems awkward. Still an allocation, and you can't avoid queueing because somebody might then a Promise after it's completed. So we can't avoid actually storing the callback for every call.

Why do we need to do this at all? What do you gain by using Promises here? It's a bit of a "fringe" feature anyway that should be used for corner cases. Normally you should use lifecycle methods. Why add more complexity to it?

Why do we need to do this at all? What do you gain by using Promises here? It's a bit of a "fringe" feature anyway that should be used for corner cases. Normally you should use lifecycle methods. Why add more complexity to it?

FWIW, my vote is to not return a Promise from setState. All of the concerns you've mentioned are good, concrete reasons.

Thanks, @gaearon and @aweary. I wasn't fully aware that you could use setState like so:

this.setState((prevState, props) => ({
  counter: prevState.counter + props.increment
}));

+1 for setState returning a promise. Consider this scenario:

functionA() sets state of someArray
functionB() sets state of other values in someArray

They are split into two functions due to business logic reasons. Sometimes A happens, sometimes B happens, sometimes both happen.

When A and B both happen and both functions are called, a race condition ensues. functionA mutates someArray and setState takes some time to update it. functionB comes in, gets the stale value of someArray and stomps over functionA's changes. Other times B happens first, immediately followed by A.
The current work-arounds are not ideal: callback hell, or smash both A and B into one monolithic function.

@peacechen

There is already an API for this use case: this.setState(fn). The function will receive the current (never stale) values of the state and props as arguments, and should return the next state. It is safe to call setState(fn) several times.

For example:

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

function multiply(prevState) {
  return {
    value: prevState.value * 5
  }
}

// inside the component
this.setState(increment);
this.setState(multiply);

@gaearon Thanks, I am aware of the callback that setState supports. My request is to add support for promises to setState to avoid callback hell.

Just want to join the chorus of and agree that at least returning a Promise on setState would be nice to have because there are times you don't want to use componentDidUpdate because it could make your component render recursively for-ev-er.

For now I'm:

promisedSetState = (newState) => {
    return new Promise((resolve) => {
        this.setState(newState, () => {
            resolve()
        }
    }
}

@gaearon I think the react team is missing why this issue is so popular.

Put yourself in the shoes of a JS developer learning ES6 and all the new libraries, Express, Gulp, etc. They've just started to understand the beauty of futures, streams, and async/await and take pride in 'knowing' how to avoid 'callback hell' which so many of their peers decry as the downfall of JS.

Then they start using React heavily, and find out that a core async part of the API uses callbacks! Which they were just done learning about how bad they are!

This one small issue is going to keep being a major pain point for new React developers, until either A) Promises are no longer used ubiquitously B) setState is longer used asynchronously or C) React returns a Promise from setState.

There are working examples of libraries that have been enhanced to support promises while retaining backwards compatibility with callbacks. In the case of setState, if a callback param is provided, it would call that as usual. However if no callback is passed in, setState would return a promise. The change is straight-forward, similar to @ctrlaltdylan 's snippet with an added check for the callback param.

async () => {
  await this.setState({ ... });
  // do something
}

it works to me.

@riverleo That's interesting. I understood that async/await works on Promise objects (it's syntactic sugar for Promise .then()). Can you confirm that it's actually waiting for the setState to complete before executing your code?

That pattern works for the simple case, but there are many times where I have a function that calls setState within, and I want to wait for that function to complete the setState call. For example:

function myFunction() {
  // Do something
  this.setState({...});
}

// would like to do this to execute after setState completes:
myFunction.then(...);

At the end of the day, we're trying to find work-arounds for a fundamental deficiency in setState(). The "right" way would be to add Promise capability to it.

@peacechen The code below is the contents of the final build of the babel.

var _ref2 = (0, _asyncToGenerator3.default)(_regenerator2.default.mark(function _callee(paginate) {
  return _regenerator2.default.wrap(function _callee$(_context) {
    while (1) {
      switch (_context.prev = _context.next) {
      case 0:
        _context.next = 2;
        return _this.setState({ ... });

      case 2:
        // do something

      case 3:
      case 'end':
        return _context.stop();
      }
    }
  }, _callee, _this2);
}));

And this is my .babelrc setting. specifically, the babel-plugin-syntax-async-generators plug-in included in the stage-0 configuration enables this.

{
  "presets": [
    "flow",
    "next/babel",
    "es2015",
    "stage-0",
  ],
  "plugins": [
    "transform-runtime",
    "transform-flow-strip-types",
    ["styled-components", { "ssr": true, "displayName": false, "preprocess": true } ],
    ["react-intl", { "messagesDir": "./locale/.messages/" }],
  ],
}

I checked the log to see if it worked, and it worked as expected with no problems to me.

@riverleo await wraps the expression in Promise.resolve. This is not correct, as the promise is not resolving once the state has updated.

@riverleo , I agree with @milesj , the generated code shows that it's not properly waiting for setState to complete. Your app just happens to finish setState quickly in that case, but will not for all cases.
Line 7 of the generated code is the key:

return _this.setState({ ... });

The correct way would be

_this.setState({ ... }, () => resolve returnVal);

(the code around that would be different, for example resolve would need to be present from a created Promise).

Created PR #9989 that returns a promise if no callback param is provided to setState. This was much simpler than diving into React's callback queue. Modifying the queue would require re-architecting and a significant rewrite.

Wrote more thoughts on why I think we won't be supporting this in https://github.com/facebook/react/pull/9989#issuecomment-309141521.

It is not very plausible that we'll change this anytime soon for these reasons so I'll close the issue.

We do, however, plan to look into providing better component APIs for components and state next year. So we'll consider this feedback as part of our future work.

Thanks!

I look forward to seeing this added next year. It is a common paradigm to support both callbacks and promises but usually not just callbacks on 'up-to-date' libraries.

The situation for having dependent set states does exist (and IMO not just an anti-pattern) which is why the callback is there in the first place.

Tossing stuff in componentDidUpdate certainly does "get around" this but you also end up with "cause here" & "effect somewhere else" sprinkled all over your React component rather than right next to each other creating a bit of a maintenance issue (at least a little confusing I'd say to anyone coming back and looking at the code next week)

I use this code but get this error:
ExceptionsManager.js:65 Cannot read property 'then' of undefined
```
componentDidMount() {
FirebaseApp.database().ref('users').on("value",(snap)=>{
var items = [];
snap.forEach((child)=>{
items.push({
name:child.val().username
})
})
this.setState({
users:items
}).then(()=>{
users2 = this.state.users.map((item)=>{
{item}

   })
  });

})
```

pls help me.

@milad1367 As far as I know, this.setState does not return a promise, so you can't chain a .then unless you Promisify it. You're better off using the second argument, or callback function.

this.setState({
    users:items
}, () => {
    let users2 = this.state.users.map((item)=>{
        <Text> {item} </Text>
    });
});

tnx.i following these instructions.

I have a question regarding setState and why I end up using the callback function often.

My understanding is that setState is asyncronous. In my product we do a lot of async ajax calls. While the call is happening I setState({isFetching: true}); This value is used to determine if the user sees a spinner or fetched data. When the request is complete and the data is parsed, the value is changed to false, which then tells the render function to display the data and not a spinner.

Within my fetch() function, which submits the request or refreshes the data I typically have a scheme like this:

this.setState({
    isFetching: true,
}, () => {
    $.ajax({
        url: '/data'
    })
    .done((data) => {
        this.setState({
            isFetching: false,
            data
        });
   });
});

This seems to me to be a common use case for using the callback in setState. I know I have seen people do things like this instead:

this.setState({
    isFetching: true,
});

$.ajax({
    url: '/data'
})
.done((data) => {
    this.setState({
        isFetching: false,
        data
    });
});

But because of the async nature of setState can we ever truly be sure the first setState won't finalize after the second (yes in theory this will almost never happen but is it still possible?)

I look forward to seeing this added next year.

We're not adding this next year (as I said above). I said that we'll consider this feedback as we think about next APIs, not that we'll make setState return a Promise.

Tossing stuff in componentDidUpdate certainly does "get around" this but you also end up with "cause here" & "effect somewhere else" sprinkled all over your React component rather than right next to each other creating a bit of a maintenance issue (at least a little confusing I'd say to anyone coming back and looking at the code next week)

In my experience, it鈥檚 the opposite pattern that becomes a maintenance burden. Somebody adds a setState callback in one place because that鈥檚 where the state gets set. Then somebody else calls setState in another place but doesn鈥檛 realize that the component鈥檚 correctness depends on that callback code running after the state change.

Putting this code in componentDidUpdate turns your component into something closer to a "state machine". The component behaves more predictably when the side effects are based on explicit conditions (e.g. a prop or a state field has changed) rather than on where in the code you happened to call setState. I understand it is a little bit frustrating if that鈥檚 not a programming model you鈥檙e used to, but I encourage you to give this a try.

The situation for having dependent set states does exist (and IMO not just an anti-pattern) which is why the callback is there in the first place.

Yes, it exists, and it is very rare (e.g. setting a focus or triggering an animation). However, for the case where it is necessary you want all the flexibility. Promises are not flexible as they force execution to happen on next tick. We could return a "thenable" object that looks like a Promise. But that's not what this issue was proposing. We recently set up an RFC process for proposing changes to React. You're welcome to propose this: https://github.com/reactjs/rfcs.

But because of the async nature of setState can we ever truly be sure the first setState won't finalize after the second (yes in theory this will almost never happen but is it still possible?)

No, it's not ever possible, React keeps an internal queue of setState calls to prevent exactly such problems. Later calls always override earlier calls. It is always safe to do as you show in the second example. You don't need to wait for setState to flush to start your API call.

I'm going to lock this issue because discussing in closed issues is not very productive. As I mentioned earlier in this comment, if you feel strongly that this is a valuable proposal despite its drawbacks, you are welcome to submit an RFC: https://github.com/reactjs/rfcs. Thank you!

Was this page helpful?
0 / 5 - 0 ratings