Skip to content
This repository has been archived by the owner on Oct 14, 2018. It is now read-only.

Batching updates #33

Closed
maberer opened this issue Jan 25, 2015 · 6 comments
Closed

Batching updates #33

maberer opened this issue Jan 25, 2015 · 6 comments

Comments

@maberer
Copy link

maberer commented Jan 25, 2015

Are batched updates on the roadmap?

If I understand it correctly, the react batching method for multiple setState() does not
work when called within an ajax request response (or things like setTimeout()).

If multiple updates to a cursor should happen in a ajax response, all calls to
setState would therefore trigger react's render algorithm...

Isn't this a problem?

@dustingetz
Copy link
Owner

I consider this a defect in React, not this cursor implementation, do you disagree?

See facebook/react#956, which if you click around the linked issues, it would seem that this was addressed in React 0.12 though I have not attempted to verify. Regardless it seems to be on React's roadmap.

@dustingetz
Copy link
Owner

If you are looking for an immediate solution, you can leverage React's immutability helpers directly (which these cursors are implemented in terms of), and set the cursor only once.

@maberer
Copy link
Author

maberer commented Jan 26, 2015

thanks for your answer.

Well it would be nice to work out of the box but I have no idea how hard this one is to fix in the context of react... I followed this discussion https://groups.google.com/forum/#!topic/reactjs/G6pljvpTGX0
and tested the script with a current version of react 0.12 - the one that seems to
have the fix implemented: http://jsfiddle.net/tindli/x7ky0v1x/1/
It always seems to execute every setState() immediately instead of batching them... so not
really seeing any change in behaviour.

In case of a naive implementation wouldn't it be enough for react-cursor to allow
taking a callback for an batchOperations() method where one can write down all
cursor manipulations to be executed before the library internally calls setState()?

cursor.batchOperations(function(cursor){
cursor.refine('a').set(1);
cursor.refine('b').refine('foo').set({ 'bar': 43, 'baz': ['red', 'green'] });
});

As you said, directly invoking immutability helpers before the last update would be another way...

Regarding the internal call to setState():
One other way would be to never ever call setState() by react-cursor directly...
I found it to be a bit hard when implementing flux where traditionally a component
is notified by a changed store; the component in charge then fetches its latest data...
Letting the store changes propagate through react-cursor would disregard the concepts of flux...

This would also mean that a cursor can be build independently from a
react component (so no this required when building the cursor) as it does
not need to know for which component to call setState()...

Nevertheless this might be beyond the scope of this issue... and as stated in the
documentation react-cursor is intended to be an opinionated implementation.

@dustingetz
Copy link
Owner

A completely orthogonal approach would be to re-implement cursors to not ever touch react state - just use regular values outside of react, and when something changes, call a top level React.render with props, which will be batched in all contexts. This makes it a lot harder to introduce cursors to an existing project that already uses react state, which I think are most projects that end up searching for a cursor library to cure that pain. But it completely bypasses all of these issues.

@maberer
Copy link
Author

maberer commented Jan 26, 2015

yeah I completely agree that the current implementation seems to be best for an existing react project looking for a cursor lib.

As soon as the strict concepts of flux are applied flow and state management are under tighter control of the implementation and setState() is manually triggered after stores (with immutable data) have updated their internal state.

For this purpose, cursors that do not automatically call setState() would be better suited... I guess.

@dustingetz
Copy link
Owner

Declined

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

No branches or pull requests

2 participants