Throttle invokes leadingEdge immediately after trailingEdge

[cpnota]

throttle with default options invokes the function more frequently than expected. It seems very unlikely that this is intended behavior, and it's inconsistent with underscore behavior.

Reproductions steps:

1. Call a throttled function twice
2. leadingEdge is invoked
3. Wait for the throttle period (wait)
4. trailingEdge is invoked
5. Call the throttled function again immediately afterwards
6. leadingEdge is invoked, even though trailingEdge was just invoked

The root cause is fairly obvious, and has nothing to do with setTimeout inconsistencies. shouldInvoke looks like:

  function shouldInvoke(time) {
    var timeSinceLastCall = time - lastCallTime,
        timeSinceLastInvoke = time - lastInvokeTime;

    // Either this is the first call, activity has stopped and we're at the
    // trailing edge, the system time has gone backwards and we're treating
    // it as the trailing edge, or we've hit the `maxWait` limit.
    return (lastCallTime === undefined || (timeSinceLastCall >= wait) ||
      (timeSinceLastCall < 0) || (maxing && timeSinceLastInvoke >= maxWait));
  }

timeSinceLastCall is the time since debounced was last invoked. Therefore, in the above scenario, on step 5 shouldInvoke returns true, and leadingEdge is called.

Discovered in a unit test that looks something like:

import { expect } from 'chai';
import sinon from 'sinon';
import throttle from 'lodash.throttle';

describe('lodash.throttle', () => {

	let clock;
	let invokeCount;
	let lastInvoked;

	const func = () => {
		invokeCount++;
		lastInvoked = Date.now();
	}

	const waitFor = ms => new Promise(resolve => {
		setTimeout(resolve, ms);
		clock.tick(ms);
	});


	beforeEach(() => {
		clock = sinon.useFakeTimers();
		invokeCount = 0;
		lastInvoked = undefined;
	});

	afterEach(() => clock.restore());

	it('invokes func at most once every 10 seconds', () => {
		const throttlePeriod = 10;
		const throttled = throttle(func, throttlePeriod);

		expect(invokeCount).to.equal(0);

		throttled();
		throttled();

		expect(invokeCount).to.equal(1);

		return waitFor(throttlePeriod).then(() => {
			expect(invokeCount).to.equal(2);

			throttled();

			expect(invokeCount,
				`throttled was called ${invokeCount} in ${Date.now()} ms.
				Current Time: ${Date.now()}, lastInvoked: ${lastInvoked}`
			).to.equal(2);
		});
	});
});

Also played with these to verify:
Lodash throttle fiddle: http://jsfiddle.net/sccqve9m/
Underscore throttle fiddle: http://jsfiddle.net/missinglink/19e2r2we/

[jdalton]

The root cause is fairly obvious, and has nothing to do with setTimeout inconsistencies. shouldInvoke looks like:

Nothing in debounce is fairly obvious.
Would you be up for creating a PR?

[cpnota]

Nothing in debounce is fairly obvious.

Fair point. Sure, I'll give it a shot.

[jdalton]

Moved to #3053.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.