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

Use yield instead of (timeout 0) #121

Closed
wants to merge 0 commits into from
Closed

Use yield instead of (timeout 0) #121

wants to merge 0 commits into from

Conversation

danielcompton
Copy link
Contributor

(timeout 0) resolves to using js/setTimeout which will actually take 4+ msecs in browsers. This is an eternity. The nextTick approach was proposed by Patrick O'Brien and implemented here.

Docs on nextTick from Closure library:

Fires the provided callbacks as soon as possible after the current JS execution context. setTimeout(…, 0) takes at least 4ms when called from within another setTimeout(…, 0) for legacy reasons.

This will not schedule the callback as a microtask (i.e. a task that can preempt user input or networking callbacks). It is meant to emulate what setTimeout(_, 0) would do if it were not throttled. If you desire
microtask behavior, use goog.Promise instead.

Unfortunately when I use this pull request against the Todo MVC example, and an internal Day8 project, I get this error. @pkobrien do you have any thoughts or ideas on this?

@danielcompton
Copy link
Contributor Author

Apologies @pkobrien, I think I had some stale caches which were causing conflicts. Running from a clean slate, this seems to be fine now. I need to do more testing on this, but it's not obviously broken.

@pkobrien
Copy link

No problem. Glad you have it working. I was surprised to see that little function giving you trouble. :-)

@mike-thompson-day8
Copy link
Contributor

The new router loop solves this problem. It no longer uses core.async. See 420e42a
Will be a part of v0.5.0 release.

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

Successfully merging this pull request may close these issues.

3 participants