From d5d0c3a8da34723f2691a6968c31fc7919a7698a Mon Sep 17 00:00:00 2001 From: Ed Hager Date: Mon, 30 Oct 2017 13:13:57 -0700 Subject: [PATCH 1/3] setTimeout in IE and Edge do not guarantee the delay. Often the callback will fire is less amount of time. Modify util.throttle and util.debounce so the delay is always guaranteed. --- src/util.ts | 31 +++- tests/unit/util.ts | 435 +++++++++++++++++++++++++-------------------- 2 files changed, 266 insertions(+), 200 deletions(-) diff --git a/src/util.ts b/src/util.ts index b0afb5f0..70d78671 100644 --- a/src/util.ts +++ b/src/util.ts @@ -29,15 +29,15 @@ export function createTimer(callback: (...args: any[]) => void, delay?: number): export function debounce void>(callback: T, delay: number): T { // node.d.ts clobbers setTimeout/clearTimeout with versions that return/receive NodeJS.Timer, // but browsers return/receive a number - let timer: any; + let timer: Handle | null; return function () { - timer && clearTimeout(timer); + timer && timer.destroy(); let context = this; let args: IArguments | null = arguments; - timer = setTimeout(function () { + timer = guaranteeMinimumTimeout(function () { callback.apply(context, args); args = context = timer = null; }, delay); @@ -62,7 +62,7 @@ export function throttle void>(callback ran = true; callback.apply(this, arguments); - setTimeout(function () { + guaranteeMinimumTimeout(function () { ran = null; }, delay); }; @@ -89,9 +89,30 @@ export function throttleAfter void>(cal let context = this; let args: IArguments | null = arguments; - setTimeout(function () { + guaranteeMinimumTimeout(function () { callback.apply(context, args); args = context = ran = null; }, delay); }; } + +export function guaranteeMinimumTimeout(callback: (...args: any[]) => void, delay?: number): Handle { + const startTime = Date.now(); + let timerId: number | null; + + function timeoutHandler() { + const delta = Date.now() - startTime; + if (delay == null || delta >= delay) { + callback(); + } else { + timerId = setTimeout(timeoutHandler); + } + } + timerId = setTimeout(timeoutHandler, delay); + return createHandle(() => { + if (timerId != null) { + clearTimeout(timerId); + timerId = null; + } + }); +} diff --git a/tests/unit/util.ts b/tests/unit/util.ts index 0ec0bd3a..591cf8ab 100644 --- a/tests/unit/util.ts +++ b/tests/unit/util.ts @@ -24,233 +24,278 @@ import { Handle } from '@dojo/interfaces/core'; import * as util from '../../src/util'; const TIMEOUT = 3000; +let timerHandle: any = null; registerSuite('utility functions', { - createTimer: (function () { - let timer: Handle | null; + afterEach() { + timerHandle && clearTimeout(timerHandle); + timerHandle = null; + }, - return { - afterEach() { - timer && timer.destroy(); - timer = null; + tests: { + createTimer: (function () { + let timer: Handle | null; + + return { + afterEach() { + timer && timer.destroy(); + timer = null; + }, + + destroy(this: any) { + const dfd = this.async(1000); + const spy = sinon.spy(); + timer = util.createTimer(spy, 100); + + setTimeout(function () { + if (timer) { + timer.destroy(); + } + }, 50); + + setTimeout(dfd.callback(function () { + assert.strictEqual(spy.callCount, 0); + }), 110); + }, + + timeout(this: any) { + const dfd = this.async(1000); + const spy = sinon.spy(); + timer = util.createTimer(spy, 100); + + setTimeout(dfd.callback(function () { + assert.strictEqual(spy.callCount, 1); + }), 110); + } + }; + })(), + + guaranteeMinimumTimeout: (function () { + let timer: Handle | null; + + return { + afterEach() { + timer && timer.destroy(); + timer = null; + }, + + destroy(this: any) { + const dfd = this.async(1000); + const spy = sinon.spy(); + timer = util.guaranteeMinimumTimeout(spy, 100); + + setTimeout(function () { + if (timer) { + timer.destroy(); + } + }, 50); + + setTimeout(dfd.callback(function () { + assert.strictEqual(spy.callCount, 0); + }), 110); + }, + + timeout(this: any) { + const dfd = this.async(1000); + const startTime = Date.now(); + timer = util.guaranteeMinimumTimeout(dfd.callback(function () { + const dif = Date.now() - startTime; + assert.isTrue(dif >= 100, 'Delay was ' + dif + 'ms.'); + }), 100); + }, + + 'timeout no delay'(this: any) { + const dfd = this.async(1000); + timer = util.guaranteeMinimumTimeout(dfd.callback(function () { + assert.isTrue(true); + })); + } + }; + })(), + + debounce: { + 'preserves context'(this: any) { + const dfd = this.async(TIMEOUT); + // FIXME + let foo = { + bar: util.debounce(dfd.callback(function (this: any) { + assert.strictEqual(this, foo, 'Function should be executed with correct context'); + }), 0) + }; + + foo.bar(); }, - destroy(this: any) { - const dfd = this.async(1000); - const spy = sinon.spy(); - timer = util.createTimer(spy, 100); + 'receives arguments'(this: any) { + const dfd = this.async(TIMEOUT); + const testArg1 = 5; + const testArg2 = 'a'; + const debouncedFunction = util.debounce(dfd.callback(function (a: number, b: string) { + assert.strictEqual(a, testArg1, 'Function should receive correct arguments'); + assert.strictEqual(b, testArg2, 'Function should receive correct arguments'); + }), 0); - setTimeout(function () { - if (timer) { - timer.destroy(); - } - }, 50); - - setTimeout(dfd.callback(function () { - assert.strictEqual(spy.callCount, 0); - }), 110); + debouncedFunction(testArg1, testArg2); }, - timeout(this: any) { - const dfd = this.async(1000); - const spy = sinon.spy(); - timer = util.createTimer(spy, 100); + 'debounces callback'(this: any) { + const dfd = this.async(TIMEOUT); + const debouncedFunction = util.debounce(dfd.callback(function () { + assert.isAbove(Date.now() - lastCallTick, 10, 'Function should not be called until period has elapsed without further calls'); - setTimeout(dfd.callback(function () { - assert.strictEqual(spy.callCount, 1); - }), 110); - } - }; - })(), - - debounce: { - 'preserves context'(this: any) { - const dfd = this.async(TIMEOUT); - // FIXME - var foo = { - bar: util.debounce(dfd.callback(function (this: any) { - assert.strictEqual(this, foo, 'Function should be executed with correct context'); - }), 0) - }; - - foo.bar(); - }, + // Typically, we expect the 3rd invocation to be the one that is executed. + // Although the setTimeout in 'run' specifies a delay of 5ms, a very slow test environment may + // take longer. If 25+ ms has actually elapsed, then the first or second invocation may end up + // being eligible for execution. + }), 25); - 'receives arguments'(this: any) { - const dfd = this.async(TIMEOUT); - const testArg1 = 5; - const testArg2 = 'a'; - const debouncedFunction = util.debounce(dfd.callback(function (a: number, b: string) { - assert.strictEqual(a, testArg1, 'Function should receive correct arguments'); - assert.strictEqual(b, testArg2, 'Function should receive correct arguments'); - }), 0); + let runCount = 1; + let lastCallTick: number; - debouncedFunction(testArg1, testArg2); - }, + function run() { + lastCallTick = Date.now(); + debouncedFunction(); + runCount += 1; - 'debounces callback'(this: any) { - const dfd = this.async(TIMEOUT); - const debouncedFunction = util.debounce(dfd.callback(function () { - assert.isAbove(Date.now() - lastCallTick, 10, 'Function should not be called until period has elapsed without further calls'); - - // Typically, we expect the 3rd invocation to be the one that is executed. - // Although the setTimeout in 'run' specifies a delay of 5ms, a very slow test environment may - // take longer. If 25+ ms has actually elapsed, then the first or second invocation may end up - // being eligible for execution. - // If the first or second invocation has been called there's no need to let the run loop continue. - clearTimeout(handle); - }), 25); - - let runCount = 1; - let lastCallTick: number; - let handle: any; - - function run() { - lastCallTick = Date.now(); - debouncedFunction(); - runCount += 1; - - if (runCount < 4) { - handle = setTimeout(run, 5); + if (runCount < 4) { + timerHandle = setTimeout(run, 5); + } } - } - - run(); - } - }, - throttle: { - 'preserves context'(this: any) { - const dfd = this.async(TIMEOUT); - // FIXME - var foo = { - bar: util.throttle(dfd.callback(function (this: any) { - assert.strictEqual(this, foo, 'Function should be executed with correct context'); - }), 0) - }; - - foo.bar(); + run(); + } }, - 'receives arguments'(this: any) { - const dfd = this.async(TIMEOUT); - const testArg1 = 5; - const testArg2 = 'a'; - const throttledFunction = util.throttle(dfd.callback(function (a: number, b: string) { - assert.strictEqual(a, testArg1, 'Function should receive correct arguments'); - assert.strictEqual(b, testArg2, 'Function should receive correct arguments'); - }), 0); - - throttledFunction(testArg1, testArg2); - }, + throttle: { + 'preserves context'(this: any) { + const dfd = this.async(TIMEOUT); + // FIXME + const foo = { + bar: util.throttle(dfd.callback(function(this: any) { + assert.strictEqual(this, foo, 'Function should be executed with correct context'); + }), 0) + }; + + foo.bar(); + }, - 'throttles callback'(this: any) { - const dfd = this.async(TIMEOUT); - // FIXME - - let callCount = 0; - let cleared = false; - const throttledFunction = util.throttle(dfd.rejectOnError(function (a: string) { - callCount++; - assert.notStrictEqual(a, 'b', 'Second invocation should be throttled'); - // Rounding errors? - // Technically, the time diff should be greater than 24ms, but in some cases - // it is equal to 24ms. - assert.isAbove(Date.now() - lastRunTick, 23, - 'Function should not be called until throttle delay has elapsed'); - - lastRunTick = Date.now(); - if (callCount > 1) { - clearTimeout(handle); - cleared = true; - dfd.resolve(); - } - }), 25); + 'receives arguments'(this: any) { + const dfd = this.async(TIMEOUT); + const testArg1 = 5; + const testArg2 = 'a'; + const throttledFunction = util.throttle(dfd.callback(function (a: number, b: string) { + assert.strictEqual(a, testArg1, 'Function should receive correct arguments'); + assert.strictEqual(b, testArg2, 'Function should receive correct arguments'); + }), 0); - let runCount = 1; - let lastRunTick = 0; - let handle: any; + throttledFunction(testArg1, testArg2); + }, - function run() { - throttledFunction('a'); - throttledFunction('b'); - runCount += 1; + 'throttles callback'(this: any) { + const dfd = this.async(TIMEOUT); + let callCount = 0; + let cleared = false; + let handle: Handle | null; + const throttledFunction = util.throttle(dfd.rejectOnError(function (a: string) { + callCount++; + assert.notStrictEqual(a, 'b', 'Second invocation should be throttled'); + // Rounding errors? + // Technically, the time diff should be greater than 24ms, but in some cases + // it is equal to 24ms. + assert.isAbove(Date.now() - lastRunTick, 23, + 'Function should not be called until throttle delay has elapsed'); + + lastRunTick = Date.now(); + if (callCount > 1) { + handle && handle.destroy(); + cleared = true; + dfd.resolve(); + } + }), 25); - if (runCount < 10 && !cleared) { - handle = setTimeout(run, 5); - } - } + let runCount = 1; + let lastRunTick = 0; - run(); - assert.strictEqual(callCount, 1, - 'Function should be called as soon as it is first invoked'); - } - }, + function run() { + throttledFunction('a'); + throttledFunction('b'); + runCount += 1; - throttleAfter: { - 'preserves context'(this: any) { - const dfd = this.async(TIMEOUT); - // FIXME - var foo = { - bar: util.throttleAfter(dfd.callback(function(this: any) { - assert.strictEqual(this, foo, 'Function should be executed with correct context'); - }), 0) - }; + if (runCount < 10 && !cleared) { + handle = util.guaranteeMinimumTimeout(run, 5); + } + } - foo.bar(); + run(); + assert.strictEqual(callCount, 1, + 'Function should be called as soon as it is first invoked'); + } }, - 'receives arguments'(this: any) { - const dfd = this.async(TIMEOUT); - const testArg1 = 5; - const testArg2 = 'a'; - const throttledFunction = util.throttleAfter(dfd.callback(function (a: number, b: string) { - assert.strictEqual(a, testArg1, 'Function should receive correct arguments'); - assert.strictEqual(b, testArg2, 'Function should receive correct arguments'); - }), 0); + throttleAfter: { + 'preserves context'(this: any) { + const dfd = this.async(TIMEOUT); + // FIXME + const foo = { + bar: util.throttleAfter(dfd.callback(function(this: any) { + assert.strictEqual(this, foo, 'Function should be executed with correct context'); + }), 0) + }; + + foo.bar(); + }, - throttledFunction(testArg1, testArg2); - }, + 'receives arguments'(this: any) { + const dfd = this.async(TIMEOUT); + const testArg1 = 5; + const testArg2 = 'a'; + const throttledFunction = util.throttleAfter(dfd.callback(function (a: number, b: string) { + assert.strictEqual(a, testArg1, 'Function should receive correct arguments'); + assert.strictEqual(b, testArg2, 'Function should receive correct arguments'); + }), 0); - 'throttles callback'(this: any) { - const dfd = this.async(TIMEOUT); - // FIXME - let callCount = 0; - let cleared = false; - const throttledFunction = util.throttle(dfd.rejectOnError(function (a: string) { - callCount++; - assert.notStrictEqual(a, 'b', 'Second invocation should be throttled'); - // Rounding errors? - // Technically, the time diff should be greater than 24ms, but in some cases - // it is equal to 24ms. - assert.isAbove(Date.now() - lastRunTick, 23, - 'Function should not be called until throttle delay has elapsed'); - - lastRunTick = Date.now(); - if (callCount > 1) { - clearTimeout(handle); - cleared = true; - dfd.resolve(); - } - }), 25); + throttledFunction(testArg1, testArg2); + }, - let runCount = 1; - let lastRunTick = 0; - let handle: any; + 'throttles callback'(this: any) { + const dfd = this.async(TIMEOUT); + // FIXME + let callCount = 0; + let cleared = false; + const throttledFunction = util.throttle(dfd.rejectOnError(function (a: string) { + callCount++; + assert.notStrictEqual(a, 'b', 'Second invocation should be throttled'); + // Rounding errors? + // Technically, the time diff should be greater than 24ms, but in some cases + // it is equal to 24ms. + assert.isAbove(Date.now() - lastRunTick, 23, + 'Function should not be called until throttle delay has elapsed'); + + lastRunTick = Date.now(); + if (callCount > 1) { + clearTimeout(timerHandle); + cleared = true; + dfd.resolve(); + } + }), 25); + + let runCount = 1; + let lastRunTick = 0; - function run() { - throttledFunction('a'); - throttledFunction('b'); - runCount += 1; + function run() { + throttledFunction('a'); + throttledFunction('b'); + runCount += 1; - if (runCount < 10 && !cleared) { - handle = setTimeout(run, 5); + if (runCount < 10 && !cleared) { + timerHandle = setTimeout(run, 5); + } } - } - run(); - assert.strictEqual(callCount, 1, - 'Function should be called as soon as it is first invoked'); + run(); + assert.strictEqual(callCount, 1, + 'Function should be called as soon as it is first invoked'); + } } } }); From b7e0fc646c8cca64336c76d45c1ad20dd3fc980a Mon Sep 17 00:00:00 2001 From: Ed Hager Date: Mon, 30 Oct 2017 16:32:28 -0700 Subject: [PATCH 2/3] Add missing delta parameter to timeout call. Fix tests. --- src/util.ts | 4 +- tests/unit/util.ts | 192 ++++++++++++++++++++------------------------- 2 files changed, 89 insertions(+), 107 deletions(-) diff --git a/src/util.ts b/src/util.ts index 70d78671..e1ca7568 100644 --- a/src/util.ts +++ b/src/util.ts @@ -105,7 +105,9 @@ export function guaranteeMinimumTimeout(callback: (...args: any[]) => void, dela if (delay == null || delta >= delay) { callback(); } else { - timerId = setTimeout(timeoutHandler); + // Cast setTimeout return value to fix TypeScript parsing bug. Without it, + // it thinks we are using the Node version of setTimeout. + timerId = setTimeout(timeoutHandler, delta); } } timerId = setTimeout(timeoutHandler, delay); diff --git a/tests/unit/util.ts b/tests/unit/util.ts index 591cf8ab..d55d3938 100644 --- a/tests/unit/util.ts +++ b/tests/unit/util.ts @@ -24,94 +24,85 @@ import { Handle } from '@dojo/interfaces/core'; import * as util from '../../src/util'; const TIMEOUT = 3000; -let timerHandle: any = null; +let timerHandle: Handle | null; + +function destroyTimerHandle() { + if (timerHandle) { + timerHandle.destroy(); + timerHandle = null; + } +} registerSuite('utility functions', { afterEach() { - timerHandle && clearTimeout(timerHandle); - timerHandle = null; + destroyTimerHandle(); }, tests: { - createTimer: (function () { - let timer: Handle | null; - - return { - afterEach() { - timer && timer.destroy(); - timer = null; - }, - - destroy(this: any) { - const dfd = this.async(1000); - const spy = sinon.spy(); - timer = util.createTimer(spy, 100); - - setTimeout(function () { - if (timer) { - timer.destroy(); - } - }, 50); - - setTimeout(dfd.callback(function () { - assert.strictEqual(spy.callCount, 0); - }), 110); - }, - - timeout(this: any) { - const dfd = this.async(1000); - const spy = sinon.spy(); - timer = util.createTimer(spy, 100); - - setTimeout(dfd.callback(function () { - assert.strictEqual(spy.callCount, 1); - }), 110); - } - }; - })(), - - guaranteeMinimumTimeout: (function () { - let timer: Handle | null; - - return { - afterEach() { - timer && timer.destroy(); - timer = null; - }, - - destroy(this: any) { - const dfd = this.async(1000); - const spy = sinon.spy(); - timer = util.guaranteeMinimumTimeout(spy, 100); - - setTimeout(function () { - if (timer) { - timer.destroy(); - } - }, 50); - - setTimeout(dfd.callback(function () { - assert.strictEqual(spy.callCount, 0); - }), 110); - }, - - timeout(this: any) { - const dfd = this.async(1000); - const startTime = Date.now(); - timer = util.guaranteeMinimumTimeout(dfd.callback(function () { - const dif = Date.now() - startTime; - assert.isTrue(dif >= 100, 'Delay was ' + dif + 'ms.'); - }), 100); - }, - - 'timeout no delay'(this: any) { - const dfd = this.async(1000); - timer = util.guaranteeMinimumTimeout(dfd.callback(function () { - assert.isTrue(true); - })); - } - }; - })(), + createTimer: { + destroy(this: any) { + const dfd = this.async(1000); + const spy = sinon.spy(); + timerHandle = util.createTimer(spy, 100); + + setTimeout(function () { + destroyTimerHandle(); + }, 50); + + setTimeout(dfd.callback(function () { + assert.strictEqual(spy.callCount, 0); + }), 110); + }, + + timeout(this: any) { + const dfd = this.async(1000); + const spy = sinon.spy(); + timerHandle = util.createTimer(spy, 100); + + setTimeout(dfd.callback(function () { + assert.strictEqual(spy.callCount, 1); + }), 110); + } + }, + + guaranteeMinimumTimeout: { + destroy(this: any) { + const dfd = this.async(1000); + const spy = sinon.spy(); + timerHandle = util.guaranteeMinimumTimeout(spy, 100); + + setTimeout(function () { + destroyTimerHandle(); + }, 50); + + setTimeout(dfd.callback(function () { + assert.strictEqual(spy.callCount, 0); + }), 110); + }, + + timeout(this: any) { + const dfd = this.async(1000); + const startTime = Date.now(); + timerHandle = util.guaranteeMinimumTimeout(dfd.callback(function () { + const dif = Date.now() - startTime; + assert.isTrue(dif >= 100, 'Delay was ' + dif + 'ms.'); + }), 100); + }, + + 'timeout no delay'(this: any) { + const dfd = this.async(1000); + timerHandle = util.guaranteeMinimumTimeout(dfd.callback(function () { + // test will timeout if not called + })); + }, + + 'timeout zero delay'(this: any) { + const dfd = this.async(1000); + timerHandle = util.guaranteeMinimumTimeout(dfd.callback(function () { + // test will timeout if not called + }), 0); + } + }, debounce: { 'preserves context'(this: any) { @@ -158,7 +149,7 @@ registerSuite('utility functions', { runCount += 1; if (runCount < 4) { - timerHandle = setTimeout(run, 5); + setTimeout(run, 5); } } @@ -171,7 +162,7 @@ registerSuite('utility functions', { const dfd = this.async(TIMEOUT); // FIXME const foo = { - bar: util.throttle(dfd.callback(function(this: any) { + bar: util.throttle(dfd.callback(function (this: any) { assert.strictEqual(this, foo, 'Function should be executed with correct context'); }), 0) }; @@ -195,7 +186,6 @@ registerSuite('utility functions', { const dfd = this.async(TIMEOUT); let callCount = 0; let cleared = false; - let handle: Handle | null; const throttledFunction = util.throttle(dfd.rejectOnError(function (a: string) { callCount++; assert.notStrictEqual(a, 'b', 'Second invocation should be throttled'); @@ -207,7 +197,7 @@ registerSuite('utility functions', { lastRunTick = Date.now(); if (callCount > 1) { - handle && handle.destroy(); + destroyTimerHandle(); cleared = true; dfd.resolve(); } @@ -222,7 +212,7 @@ registerSuite('utility functions', { runCount += 1; if (runCount < 10 && !cleared) { - handle = util.guaranteeMinimumTimeout(run, 5); + timerHandle = util.guaranteeMinimumTimeout(run, 5); } } @@ -237,7 +227,7 @@ registerSuite('utility functions', { const dfd = this.async(TIMEOUT); // FIXME const foo = { - bar: util.throttleAfter(dfd.callback(function(this: any) { + bar: util.throttleAfter(dfd.callback(function (this: any) { assert.strictEqual(this, foo, 'Function should be executed with correct context'); }), 0) }; @@ -259,42 +249,32 @@ registerSuite('utility functions', { 'throttles callback'(this: any) { const dfd = this.async(TIMEOUT); - // FIXME + let callCount = 0; - let cleared = false; - const throttledFunction = util.throttle(dfd.rejectOnError(function (a: string) { + let lastRunTick = 0; + const throttledFunction = util.throttleAfter(dfd.rejectOnError(function (a: string) { callCount++; assert.notStrictEqual(a, 'b', 'Second invocation should be throttled'); - // Rounding errors? - // Technically, the time diff should be greater than 24ms, but in some cases - // it is equal to 24ms. assert.isAbove(Date.now() - lastRunTick, 23, 'Function should not be called until throttle delay has elapsed'); lastRunTick = Date.now(); - if (callCount > 1) { - clearTimeout(timerHandle); - cleared = true; + if (callCount > 2) { + destroyTimerHandle(); dfd.resolve(); } }), 25); - let runCount = 1; - let lastRunTick = 0; - function run() { throttledFunction('a'); throttledFunction('b'); - runCount += 1; - if (runCount < 10 && !cleared) { - timerHandle = setTimeout(run, 5); - } + timerHandle = util.guaranteeMinimumTimeout(dfd.rejectOnError(run), 5); } run(); - assert.strictEqual(callCount, 1, - 'Function should be called as soon as it is first invoked'); + assert.strictEqual(callCount, 0, + 'Function should not be called as soon as it is first invoked'); } } } From 7993bf432baf1faff40de30e73457da4c0b2a149 Mon Sep 17 00:00:00 2001 From: Ed Hager Date: Fri, 3 Nov 2017 08:40:53 -0700 Subject: [PATCH 3/3] Fix guaranteeMinimumTimeout timer delta calculation. --- src/util.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util.ts b/src/util.ts index e1ca7568..f84a48b5 100644 --- a/src/util.ts +++ b/src/util.ts @@ -107,7 +107,9 @@ export function guaranteeMinimumTimeout(callback: (...args: any[]) => void, dela } else { // Cast setTimeout return value to fix TypeScript parsing bug. Without it, // it thinks we are using the Node version of setTimeout. - timerId = setTimeout(timeoutHandler, delta); + // Revisit this with the next TypeScript update. + // Set another timer for the mount of time that we came up short. + timerId = setTimeout(timeoutHandler, delay - delta); } } timerId = setTimeout(timeoutHandler, delay);