-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
Strategy for avoiding cascading renders #125
Comments
Thanks for posting this. Keeping this open for now. I'll revisit after ReactEurope. |
If you run into someone from Relay it could be good to get their input since this is the kind of performance problem faced by a big website with many components reading from the same stores. |
@ide some good thoughts there, it would definitely be a welcome improvement, as performance increases are always a good thing :) |
Related: facebook/react#3920 (comment) |
Can't we just wrap the subscription notification in |
@ide that's what you suggested to me the other day in regard to flummox, right? |
I don't know the behavior of batchedUpdates across components though it may solve this problem. Will have to look at its source. In flummox's case one component might subscribe to multiple stores that each emit a change in response to the same action, so batching that component's setState calls is useful. With redux components have just one subscription to the redux instance and all the stores update before the component is notified once per action. |
It reaches into React's internals and tells it that Isn't this the kind of thing you were talking about? Both parent and child are invalidated, but the DOM changes are not cascading and instead applied in one pass. Perhaps @spicyj can confirm if that's indeed how |
Yes, that's what batchedUpdates does. Parents will reconcile first so that the children render only once (with the newest props from their parent and their newest state). |
Thanks! The un-nice thing about it is that it's an addon, which means we can't get it from I wonder if 0.14 solves that somehow. |
You'll be able to require |
Can a library depend on anything from |
That's right, it wouldn't. |
It's unfortunate |
We think of it slightly differently – addons use React internals (which is why we package them at all instead of letting someone else write it), but their API is liable to change more frequently. |
OK, thanks for explaining! |
Cool - sounds like batchedUpdates addresses this then. I made a little demo with two nested components that read the same store data and to my surprise the updates already appear batched (the child gets its new props and new state together). https://gist.github.com/ide/1da8a70a14535835da30 |
@gaearon Would there be any downside in a middleware that wraps all dispatches in batchedUpdates? |
@acdlite I don't think so, that's what I'd do. |
@gaearon You're right -- React Native wasn't the best environment to test in because it batches messages from the bridge :P |
@ide React will also batch for DOM events. But it won't for AJAX callbacks etc. |
NuclearJS just added a |
There are some unsolved issues with batching such as with controlled components. The plan is likely to move to batching by default with an opt-out for difficult cases. |
@sebmarkbage Great to hear, that sounds sensible. Closing this since the extension I posted above addresses the issue. |
On second thought let's keep this open until we have docs surrounding it. |
Added to docs in 13058f0. |
@ide @gaearon i know this is closed but for educational purposes what are the reasons someone would want to connect the child components rather than push the root parent state as props down the component tree? This way only the root parent is connected. This is heirarchical composition, so descendants should all be stateless components, no? For example, a calendar date selection component would have one stateful parent and a bunch of stateless components as descendants. Connected state is pushed down via read only props from calendar elemt root component to the statelsss descendants. What am I missing? |
I think of it as a matter of grouping related code or concerns together. In contrast, consider an approach like Relay's where many components are connected and the components and their data requirements (ex: a Redux selector or Relay query) exhibit high cohesion. Imagine if a site like GitHub were implemented with Redux and React. The component hierarchy would likely be many levels deep. For example, the IssueCommentHeader component would be quite far away from the root component. If only the root component were connected to the store, the root would need to know that the IssueCommentHeader—very far away—needs the user's username and profile picture. What if you change IssueCommentHeader so that it needs the user's online/offline presence, like a chat app? Now you need to modify IssueCommentHeader's owner, and its owner, and so on until you've modified every component up to the root so that it threads the user's presence data down this deep component hierarchy. In summary it's not scalable for your productivity, let alone a team's productivity, to put all of the data-retrieving functionality in the root component. This was a real problem when building the liking & commenting UI on Facebook—it looks like a small component but there's a lot of complexity hiding inside it and for its root component to need to know all the data wanted by its leaves deep down was hard to write code for and maintain. What we really wanted is for components at many levels of the hierarchy to specify the data they needed so that related code was grouped together. |
I researched these questions by Dan himself (on S.O.) and users of Redux grappling with the same exact issue, so I am glad I'm not the first to ask. I think the answer on S.O. is not ideal... leaving it to the programmer is not a cozy solution @gaearon have you any updated insight? |
As I understand it, when you update a Store it broadcasts an update to all Connectors. Each Connector selects the slice of State that its interested in and re-renders itself by calling
this.setState
. React then recursively renders the component hierarchy until it hits a nested Connector, whosecomponentShouldUpdate
method compares the old and new props (it also compares the Connector's State, which is guaranteed not to have changed because (a) it's a slice of a Store's immutable State and (b) this nested Connector hasn't calledthis.setState
). If the props have changed, React continues rendering the component hierarchy. Once React is done, the second, nested Connector receives an update from the Store, selects its slice of State, and re-renders itself. In the worst-case scenario, a linear chain ofn
Connectors could result inO(n^2)
render calls.This is a small example that demonstrates this cascading render effect. Both Connectors are subscribed to the same slice of State, and the parent passes down a prop to the child.
This is what I'm currently thinking:
this._state
. This means a Connector always renders with the most up-to-date State even if it hasn't been notified of a change yet. It also means that Store updates are atomic from the perspective of the subscribing views... currently a Connector's props may be derived from the new State while the Connector's state is a slice of the old State.this._state
. If there's no change (e.g. because we already got an up-to-date slice when the parent re-rendered this Connector) then do nothing. If there is a change then we updatethis._state
and callthis.forceUpdate
.The main thing I'm going for is the atomic Store update from the perspective of the React components. From Redux's perspective, the updates are atomic in the sense that it doesn't notify subscribers till the Stores are done transforming the State. But from the perspective of a connected component it's receiving a mix of old and new State as props, while in traditional Flux the components always read a consistent view of the Stores.
The text was updated successfully, but these errors were encountered: