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

Move Heartbeat changed listening to Roact #23

Open
LPGhatguy opened this issue Mar 12, 2018 · 3 comments
Open

Move Heartbeat changed listening to Roact #23

LPGhatguy opened this issue Mar 12, 2018 · 3 comments

Comments

@LPGhatguy
Copy link
Contributor

Right now, Store.changed only fires once per Heartbeat (or other event, I don't remember) in order to reduce the number of renders resulting from Rodux changes.

When using Rodux standalone, that doesn't make any sense -- your UI framework should be doing the change batching.

I want to implement this render batching in Roact and then remove it from Rodux.

@LPGhatguy
Copy link
Contributor Author

After lots of meetings about Roact and Rodux performance, I'm less certain about this!

Roact definitely should be deduplicating changes that come from Rodux, but I'm unsure about removing the feature from Rodux still. There are definitely some cases where Rodux not firing changed synchronously can cause weird delays!

@AmaranthineCodices
Copy link
Contributor

I think there's some arguments in favor of firing Store.changed on a regular basis. There are some Redux libraries that aim to reduce the amount of subscriber invocations or batch actions together:

These are all centered around the same core idea: reducing the number of subscriber invocations. I think this is a pretty reasonable feature to build into Rodux by default, for performance reasons if nothing else.

@headjoe3
Copy link

headjoe3 commented Dec 8, 2019

Just want to let you know, there were times in which I considered using Rodux in non-Roact contexts, but decided against it for this very reason. I would re-consider moving this behavior to Roact at the very least, as this is not how regular Redux behaves.

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

No branches or pull requests

3 participants