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

Consider using unstable batched updates? #1091

Closed
markerikson opened this issue Nov 26, 2018 · 9 comments
Closed

Consider using unstable batched updates? #1091

markerikson opened this issue Nov 26, 2018 · 9 comments

Comments

@markerikson
Copy link
Contributor

@gaearon contacted me and suggested that we should go ahead and use React's unstable_batchedUpdates API in version 6, and that "not using it from the start was a mistake".

I haven't used it in practice myself, so I'm not sure what the tradeoffs are. The main thing I know is that it's part of ReactDOM, so I don't know how we'd handle this with React Native.

Thoughts?

/cc @timdorr , @cellog , @jimbolla .

@markerikson
Copy link
Contributor Author

@vincentriemer put together a notional example of how to use this with ReactDOM, or fall back to a noop in RN:

https://gist.github.com/vincentriemer/1c761d9d31d85e9077cf82546ef62f5b

@timdorr
Copy link
Member

timdorr commented Nov 26, 2018

Will this have any effect in 6.0? connect() is now stateless (it can probably be a function component now, too?), so there's nothing to batch. A single subscriber fires at the top level and tunnels through via context to the connected components.

Perhaps those context updates could be batched, but that's going to be internal to React. And those should probably be sliced into separate units of work anyways. I do wonder how we would be able to signal to React that some are higher priority than others, but that's still a bunch of other unstable_ APIs... 😄

@markerikson
Copy link
Contributor Author

Update batching is how React coalesces multiple setState() calls inside event handlers into a single update pass, so this is really about cutting down on the number of re-renders if you dispatch a bunch of times in a row.

That said, I'm not entirely sure where and how we would actually use it. Dan was trying to explain a bit over DM, but that's not a great medium. I've asked him to comment here tomorrow and explain in more detail.

And no, connect can't be a function component until hooks are ready. It's getting closer to being stateless, but it still needs some instance variables for the memoization selectors :

this.selectDerivedProps = makeDerivedPropsSelector()
this.selectChildElement = makeChildElementSelector()
this.renderWrappedComponent = this.renderWrappedComponent.bind(this)

@timdorr
Copy link
Member

timdorr commented Nov 26, 2018

Is it even necessary nowadays anyways? setState is no longer synchronous. If you call it multiple times within the same lifecycle method, it batches automatically. I'm not sure what function the API serves in modern React.

@saboya
Copy link

saboya commented Dec 4, 2018

Would that be set to a sane default, but user-overridable?

@markerikson
Copy link
Contributor Author

markerikson commented Dec 4, 2018

So here's my understanding of how things work:

  • If you call setState() by itself, React handles rendering pretty much right away, and multiple setState() calls in a row get handled separately
  • If you specifically call unstable_batchedUpdates(someCallback), and call setState() multiple times in that callback, React will batch the updates together and only re-render once.

So, the reason why setState() calls in React event handlers are batched is specifically because React runs your event handlers inside of an unstable_batchedUpdates() call.

The question here is if there's a good way to leverage this so that Redux dispatches outside of React event handlers can be batched together.

I've pinged @gaearon again and asked him to provide some more info.

I don't want this to hold up v6 (which we'd like to push to final today), but it'd be nice to investigate this further and maybe have a way to use it more easily in a 6.1 release or something.

The question of update priorities is somewhat related, but I don't think it's something we can really look into further until there's more progress on releasing Suspense features.

@timdorr
Copy link
Member

timdorr commented Dec 4, 2018

If you call setState() by itself, React handles rendering pretty much right away, and multiple setState() calls in a row get handled separately

I don't think this is true. setState() enqueues an update, but that only schedules an update to occur later. The only exception would be if it can get to this UnitOfWork before finishing the render loop's current timeslice.

I only see batchedUpdates() calls in relation to DOM or Native events. That would make sense because that code runs outside of the render loop. Within the render loop, you're just adding to the work queue, so it's batched via that queue.

Again, I'm not quite sure what we could be doing here. Perhaps something around the subscription? But given that happens at the Provider level, I'm not sure what there is to batch. 🤷‍♂️

@penspinner
Copy link

penspinner commented Dec 19, 2018

Coming from @markerikson Twitter post.

I think my immediate question is, does it only batch updates from multiple calls inside one batchedUpdates() callback, or does it batch across multiple batchedUpdates()?

bU( () => setState() );
bU( () => setState() );

Does that do one render pass, or two?

I believe that would depend on when those two batchedUpdate are called. If they are called in a function that React controls like componentDidMount, or an onClick from a React Element, then it would render once. However, if they are called in, say a setTimeout, then it would render twice. Created a sandbox to show differences: https://codesandbox.io/s/6yr5r9omy3.

Also, can you give an example of "batching multiple calls in async functions"?

Would appreciate any insight you can offer - probably best to comment in that issue thread.

I think the use case doesn't occur often. Usually need if I need to call two setState in an async function like setTimeout or after a Promise finishes.

class App extends React.Component {
  state = { loading: false, number: 0 };

  setRandomNumber() {
    this.setState({ number: Math.random() });
  }

  handleClick = () => {
    setTimeout(() => {
      // Force one render
      ReactDOM.unstable_batchedUpdates(() => {
        this.setRandomNumber();
        this.setState(({ loading }) => ({ loading: !loading }));
      });
    });
  };

  render() {
    console.log('App');

    return (
      <div className="App">
        <span>{this.state.loading && 'Loading...'}</span>
        <Button onClick={this.handleClick}>Random Number</Button>
        <div>{this.state.number}</div>
      </div>
    );
  }
}

Probably not that good of an example...

@timdorr
Copy link
Member

timdorr commented Jan 23, 2019

Since there hasn't been any forward movement here and Dan hasn't provided an example of what he's suggesting, I'm going to close this out for now. I'm not sure what we could do here anyways...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants