lib: fix undefined timeout regression#3331
Uh oh!
There was an error while loading. Please reload this page.
Conversation
lib/timers.js Outdated
There was a problem hiding this comment.
instead of util.isUndefined, should be probably just be msecs === undefined
indutny commented Oct 12, 2015
LGTM, cc @jasnell |
indutny added timers 
evanlucas commented Oct 12, 2015
yea, LGTM other than that |
rmgforce-pushed the fix-undefined-timeouts branch from d66d408 to e2c2b44Compare
rmg commented Oct 12, 2015
Fixed the |
indutny commented Oct 12, 2015
LGTM |
63644dd introduced a regression caused by everyone's favourite JavaScript feature: undefined < 0 === undefined >= 0. Add a case to the existing tests to cover this scenario and then add the check for undefined that makes the test pass. PR-URL: #3331 Reviewed-By: Fedor Indutny <[email protected]> Reviewed By: Evan Lucas <[email protected]>
evanlucas commented Oct 12, 2015
are we doing trailing commas? |
indutny commented Oct 12, 2015
Landed in bde32f8. Forgot to run CI :( Thank you! |
indutny commented Oct 12, 2015
Argh, I have overlooked it. No, we don't |
rmg commented Oct 12, 2015
sorry! should I bother fixing? |
63644dd introduced a regression caused by everyone's favourite JavaScript feature: undefined < 0 === undefined >= 0. Add a case to the existing tests to cover this scenario and then add the check for undefined that makes the test pass.
rmgforce-pushed the fix-undefined-timeouts branch from e2c2b44 to 1e14233Comparermg commented Oct 12, 2015
guess I probably shouldn't have force pushed the fix for the trailing comma.. oh well. |
Trott commented Oct 13, 2015
I'm impressed/terrified that this edge case surfaced so quickly. Thanks for finding/fixing. |
63644dd introduced a regression caused by everyone's favourite JavaScript feature: undefined < 0 === undefined >= 0. Add a case to the existing tests to cover this scenario and then add the check for undefined that makes the test pass. PR-URL: #3331 Reviewed-By: Fedor Indutny <[email protected]> Reviewed By: Evan Lucas <[email protected]>
jasnell commented Oct 13, 2015
Landed in v4.x in c245a19 |
Fishrock123 commented Oct 13, 2015
Oops. |
chrisabrams commented Oct 13, 2015
Thanks for this - really appreciate the quick turn around for those of us relying on this fix and using LTS! |
rmg commented Oct 13, 2015
@Trott I'm really aggressive with rolling new released out to StrongLoop's CI, and we have over 100 modules running through it.. didn't take long before we started getting some really mysterious build failures. |
8 participants
63644dd introduced a regression caused by everyone's favourite
JavaScript feature:
undefined < 0 === undefined >= 0.Add a case to the existing tests to cover this scenario and then add
the check for undefined that makes the test pass.
@indutny@jasnell this should fix at least one of the v4.2.0 regressions.