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

Wrap action functions in setTimeout? #22

Closed
dashed opened this issue Jul 30, 2014 · 8 comments · Fixed by #23
Closed

Wrap action functions in setTimeout? #22

dashed opened this issue Jul 30, 2014 · 8 comments · Fixed by #23
Milestone

Comments

@dashed
Copy link
Contributor

dashed commented Jul 30, 2014

It's a common use case that action functors are normally tied to virtual DOM event callbacks within components in react.js. EventEmitters (node.js or 3rd-party) are not async in nature, unless otherwise designed to be.

Thus, whenever an emit function is called, it's done synchronously which begin from the action to the store (and chained stores).

Action functors should be executed in fire-and-forget fashion. Thus, I think we should wrap action functions in setTimeout with 0ms delay.

@spoike thoughts?

@bripkens
Copy link
Contributor

strong +1 for this

@spoike
Copy link
Member

spoike commented Jul 30, 2014

I was under the impression they were deferred already.

I'm all 👍 for this

@spoike spoike added this to the 0.1.4 milestone Jul 30, 2014
@dashed
Copy link
Contributor Author

dashed commented Jul 30, 2014

Created PR at #23. Should we bother worrying about using requestAnimationFrame whenever it's available?

@spoike
Copy link
Member

spoike commented Jul 30, 2014

I'm pretty sure requestAnimationFrame is tied to the browser's frame rate while setTimeout(..., 0) isn't. We really want to run the actions as quickly as possible though.

Should we provide an option for using requestAnimationFrame instead if available? This could help out projects (HTML5 games perhaps) that rely on this.

@bripkens
Copy link
Contributor

Wouldn't you normally try to run your logic as fast as possible and then batch your UI updates to 60fps? If so, setTimeout(...) should suffice.

Then again, we can always add requestAnimationFrame support once somebody has a need for it.

@dashed
Copy link
Contributor Author

dashed commented Jul 30, 2014

Here is a nice polyfill for nextTick: https://github.com/medikoo/next-tick/blob/master/index.js

  1. MutationObserver is faster than setTimeout, and probably as fast as you can get in modern browsers that support it: http://caniuse.com/mutationobserver
  2. setImmediate would be used for IE10+.
  3. Finally, setTimeout as final fallback. (<= IE9)

@dashed
Copy link
Contributor Author

dashed commented Jul 30, 2014

bluebird promise library does something similar: https://github.com/petkaantonov/bluebird/blob/master/src/schedule.js

@dashed
Copy link
Contributor Author

dashed commented Jul 30, 2014

I amended my PR to use the next-tick module, and as well as switch it out via utils.nextTick.

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

Successfully merging a pull request may close this issue.

3 participants