Allow _.throttle to function correctly after the system time is updated

[blake-simpson]

I have found that if the system time is moved backwards, calls to throttle will stop executing.

To test this, if the system time is moved backwards by an hour then the remaining variable in _.throttle containing the time offset becomes a large positive integer and the throttle callback will not be called until the adjusted hour time difference has passed, instead of the expected wait time.

For this reason I suggest that an additional check is made to see if remaining exceeds the wait time which will then clear the existing timeout and return the method to a normal state.

I have tested this in a project and provided a unit test as proof.

[akre54]

This is mainly useful for things like Daylight Savings Time or going between time zones, right? Otherwise this seems like an incredibly slight edge case.

[melnikov-s]

Seems like _.debounce has the same problem. Small nitpick is that if a system clock is moved back , _.throttle will execute on the next hit, regardless if wait has passed or not.

Perhaps it's worth considering removing Date all together and coming up with a pure setTimeout solution for both _.debounce and _.throttle. The timer won't be adjusted for 100% accuracy but at least we won't have these edge cases.

[blake-simpson]

@akre54 This is one case, the project where I use this is a browser game based in many countries. Apparently players will adjust the system clock to match their local time difference to the servers.

@smelnikov Unfortunately yes, when the system time changes this fix may trigger a callback prematurely but I believe this is not a huge issue, as most wait offsets are small and after the initial callback, the timer will be cleared and state will return to normal.

This fix could also be applied to _.debounce too, if the solution is found to be acceptable.

[akre54]

This is probably too edge-case for Underscore. You might mixin your own timezone-independent throttle for your specific needs...

[jashkenas]

This is probably too edge-case for Underscore

Nah, this is a thing that can happen. Especially for long-lived single page apps that you don't close. Let's add the sanity check fix for this and for debounce.

[michaelficarra]

This additional test isn't sufficient for protection from leap seconds or when the delay time is long. Consider the case of a 24 hour delay. If it goes past a DST boundary, it may take either 23 or 25 hours, and this PR won't necessarily account for that.

[jashkenas]

When _.throttle has a delay time of 24 hours? Now that's an edge case, in the browser ;)

[michaelficarra]

You know what I mean. Any time over an hour, or in the case of a leap second, any time over a second.

[blake-simpson]

I believe the this won't be an issue, after reading ECMAScript Spec §15.9.1.1 it appears timestamps from Date.getTime() are normalised over time zones and leap seconds are ignored.

Taking this into account, I have also added a sanity check to _.debounce + a unit test for this.

[grahamscott]

It would appear that this fix also fixes issues with Sinon.js, specifically tests on debounced functions that use sinon.useFakeTimers (https://github.com/cjohansen/Sinon.JS/blob/master/lib/sinon/util/fake_timers.js)

[grahamscott]

I spoke to soon. It seems that there are random failures with useFakeTimers on debounced functions with underscore 1.6

[lfac-pt]

@grahamscott you can find more info on the fake timers issue at #1478, if you haven't seen it already.

[grahamscott]

@grahamscott you can find more info on the fake timers issue at #1478, if you haven't seen it already.

Thank you!