Promise-returning setState and forceUpdate

[jamiebuilds]

[Rendered]

This RFC specifies setState and forceUpdate to return a promise when no callback is provided.

class DataFetcher extends React.Component {
  state = {
    loading: false,
    error: null,
    data: null,
  };

  async componentDidMount() {
    await this.setState({ loading: true });
    try {
      let response = await fetch(this.props.url);
      let data = await response.json();
      await this.setState({ data, loading: false });
    } catch (error) {
      await this.setState({ error, loading: false });
    }
  };
  
  render() {
    return this.props.render(this.state);
  }
}

Note: This example doesn't rely so much on setState being applied before continuing. I can come up with a better example if people want it better demonstrated. I figured the data fetching example was easiest to understand.

[jamiebuilds]

"Yes, we’d have to tag it as a node to visit when committing changes. setState() without callback doesn’t need that." – Dan

[jamiebuilds]

Depending on how big of an impact this makes, I would consider this a big enough reason to reject this proposal outright.

[ruyasan]

It may be possible to detect if setState is declared async (which is not the same as returning a promise, true, but perhaps a good enough work-around?) - e.g. https://stackoverflow.com/questions/38508420/how-to-know-if-a-function-is-async

[alexkrolick]

Some promisified callback APIs (like the AWS SDK) use a call to .promise() to know whether to ~~~return a Promise~~~ create a Promise.

await this.setState({ loading: true }).promise(); // awaitable/thenable

[Jessidhia]

That still means that you'd have to return an object that has a promise method, and setState always has to call a hidden callback to notify the object just in case promise was called. You still have to do all the same bookkeeping; the only difference is whether you make a Promise instance or not.

[jamiebuilds]

I think the bigger concern is more:

let thenable = this.setState({ ... });
setImmediate(() => {
  thenable.then(() => {
    // ...
  });
});

What if .then isn't called immediately (but is potentially called before it would have resolved)?

[acdlite]

Same as calling this.setState(null, callback).

[taion]

Is that going to work as expected (i.e. the promise will resolve after the explicit setState finishes)? If so, could .then() always have those semantics?

If so, you could potentially only allocate at most a single thenable per component instance.

[jamiebuilds]

Same as calling this.setState(null, callback)

I don't see how that's possible unless scheduleCallback can be called after the update is already applied

[acdlite]

unless scheduleCallback can be called after the update is already applied

Bingo :D

[bpartridge]

This seems easy enough to provide in an external library, and therefore be entirely opt-in.

export const setStatePromise = (o, state) => new Promise(resolve => o.setState(state, resolve));

await setStatePromise(this, { loading: true })

// or if you want syntax similar to the RFC, but still opt-in for performance

this.setStateAsync = setStatePromise.bind(null, this) // in constructor

await this.setStateAsync({ loading: true })

We should be careful with naming conventions as well, now that this.deferSetState is coming down the pipeline as well... if someone wants to have deferSetStateAsync (or, for what it's worth, an AbstractDeferSetStateAsyncFactoryFactoryBean), that's a naming decision that shouldn't be made in the core library :)

[cloudever]

I think that proposal will cause a perfomance problems.

[sebmarkbage]

One data point here. In our concurrent mode API where you want to do one setState first at a higher priority followed by a setState at lower priority we originally proposed something like deferring a callback (or just requestIdleCallback or setTimeout). However, that introduced an unfortunate async seam that could now introduce new imperative effects (like other setStates). That in turn causes the sequence to be scheduled in a non-predictable way. Instead, we now propose the "setState(); scheduler.next(() => { setState(); })" pattern which just changes the batching/priorities but the whole thing is added to the queue synchronously so that subsequent events are stacked on top of the sequence instead of interleaving.

The original example in the beginning of this RFC is actually a good example of this. The classic problem with code like this is that it's not resilient to new updates coming in - in the meantime. E.g. what if props updates? How do fetches that happen during update interleave with other updates happening during the async sequence?

For data fetching in general, we have the whole Suspense thing which can solve most of the canonical use cases for this. For the more general problem, I think we need something that defines a sequence up front instead of triggering imperative code along the way. That way if other imperative code adds a sequence, it will by default be added in full after the first sequence instead of some operations becoming interleaved. Then those can be combined to be executed in different priorities or collapse like our update queue generally works.

We've learned a lot since this RFC was originally posted. I'm now pretty skeptical that this exact pattern is the right way forward. It's (deceiving) simplicity is aspirational for any API to compete with though.

[Niryo]

Can someone update about this RFC? This is a fairly easy and straight-forward suggestion, why is it still open?
It should have been merged already (or closed) ages ago, is there anything left to discuss here?
I am trying to figure out if this whole RFC mechanism is really working, or is it just a time waster for the community, because it looks like that not even a single RFC had been merged, that was not suggested by a member of the core team itself. All the community suggestions are just left stale.
Can someone from the React team confirm to us that this RFC process is still active and we are not wasting our time? (not this specific RFC, I am talking about the whole RFC process now).
@gaearon, @bvaughn ??

[jamiebuilds]

This rfc predates hooks, and I don't really think it's worth improving the class api anymore, I'm gonna close it