-
Notifications
You must be signed in to change notification settings - Fork 61
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
errors and schedualling #38
errors and schedualling #38
Conversation
@@ -14,5 +14,8 @@ | |||
"main": "./index.js", | |||
"engines": { | |||
"node": ">= 0.6.0" | |||
}, | |||
"devDependencies": { | |||
"tape": "^3.5.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use mocha for tests, and the dependency version must be specified explicitly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
What does this do to the performance? One of the reasons all this changed was because there was some performance benefit originally iirc. |
good question let me put together something |
https://jsperf.com/asyncstuff/8, so modestly slower, though interestingly the try/catch is not the drain that I expected it would be |
but trying to do it with try in a way that doesn't move where the uncaught error ends up AND isn't super slow in firefox is proving challenging. |
Is this the final version of the PR or is it still being investigated? |
final, try/catch had some subtle and unexpected performance issues in certain browsers but was at best the same as this performance wise |
Subscribing, this will help me out, a lot. |
if (currentQueue.length) { | ||
queue = currentQueue.concat(queue); | ||
} else { | ||
queueIndex = -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code style: 4 spaces
sorry for the delay. Just some minor comments. Please also squash the commits. |
0b795a0
to
774a338
Compare
fixed |
Thank you! I need this so badly! |
NICE! |
slightly more complete solution to #37 with tests!
Errors are not rethrown so it will still show up as an uncaught error when used with a debuger also a try/catch is avoided which can be slow in certain browsers