Merge pull request #79 from universal-ember/fix-debounce-cleanup-timing

Fix debounce cleanup timing

Author: NullVoxPopuli

reactiveweb/src/debounce.ts

@@ -60,15 +60,23 @@ class TrackedValue<T> {
  */
 export function debounce<Value = unknown>(ms: number, thunk: () => Value) {
   let lastValue: Value;
-  let timer: number;
   let state = new TrackedValue<Value>();
 
   return resource(({ on }) => {
+    let timer: number;
+
     lastValue = thunk();
 
-    on.cleanup(() => timer && clearTimeout(timer));
-    timer = setTimeout(() => (state.value = lastValue), ms);
+    on.cleanup(() => {
+      if (timer) {
+        clearTimeout(timer);
+      }
+    });
+
+    timer = setTimeout(() => {
+      state.value = lastValue;
+    }, ms);
 
-    return state.value;
+    return () => state.value;
   });
 }

tests/test-app/tests/utils/debounce/js-test.ts

@@ -9,40 +9,33 @@ import { debounce } from 'reactiveweb/debounce';
 module('Utils | debounce | js', function (hooks) {
   setupTest(hooks);
 
-  let someTime = (ms = 25) => new Promise((resolve) => setTimeout(resolve, ms));
+  const timeout = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
 
   module('debounce', function () {
-    test('works with @use', async function (assert) {
+    test('value is returned after x ms', async function (assert) {
       class Test {
-        @tracked data = '';
+        @tracked value = 'initial';
 
-        @use text = debounce(100, () => this.data);
+        @use debouncedValue = debounce(100, () => this.value);
       }
 
       let test = new Test();
 
       setOwner(test, this.owner);
 
-      assert.strictEqual(test.text, undefined);
+      assert.strictEqual(test.debouncedValue, undefined, 'Value is undefined at first');
 
-      test.data = 'b';
-      await someTime();
-      assert.strictEqual(test.text, undefined);
-      test.data = 'bo';
-      await someTime();
-      assert.strictEqual(test.text, undefined);
-      test.data = 'boo';
-      await someTime();
-      assert.strictEqual(test.text, undefined);
+      await timeout(50);
 
-      await someTime(110);
-      assert.strictEqual(test.text, 'boo');
+      assert.strictEqual(test.debouncedValue, undefined, 'Value is still undefined after ~50ms');
 
-      test.data = 'boop';
-      assert.strictEqual(test.text, 'boo');
+      await timeout(50);
 
-      await someTime(110);
-      assert.strictEqual(test.text, 'boop');
+      assert.strictEqual(
+        test.debouncedValue,
+        test.value,
+        `Value is "${test.debouncedValue}" after ~100ms`
+      );
     });
   });
 });

tests/test-app/tests/utils/debounce/rendering-test.gts

@@ -0,0 +1,55 @@
+import { tracked } from '@glimmer/tracking';
+import { setOwner } from '@ember/application';
+import { render } from '@ember/test-helpers';
+import { module, test } from 'qunit';
+import { setupRenderingTest } from 'ember-qunit';
+
+import { use } from 'ember-resources';
+import { debounce } from 'reactiveweb/debounce';
+
+module('debounce | rendering', function (hooks) {
+  setupRenderingTest(hooks);
+
+  const timeout = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
+
+  test('value is returned *after* delay has passed', async function(assert) {
+      class Test {
+        @tracked value = 'initial';
+
+        @use debouncedValue = debounce(100, () => this.value);
+      }
+
+      let test = new Test();
+
+      setOwner(test, this.owner);
+
+      await render(<template><output>{{test.debouncedValue}}</output></template>);
+
+      assert.dom('output').hasNoText();
+      assert.strictEqual(test.debouncedValue, undefined, 'Value is undefined at first');
+
+      await timeout(25);
+
+      assert.dom('output').hasNoText();
+      assert.strictEqual(test.debouncedValue, undefined, 'Value is still undefined after ~25ms');
+
+      // Changing the value resets the debounce timer
+      test.value = 'new';
+
+      // not quite enough
+      await timeout(80);
+
+      assert.dom('output').hasNoText();
+      assert.strictEqual(test.debouncedValue, undefined, `Value is still undefined after ~105ms (because we changed something at ~25ms)`);
+
+      await timeout(50);
+
+      assert.dom('output').hasText('new');
+      assert.strictEqual(test.debouncedValue, test.value, `Value is "${test.value}" after ~155ms`);
+
+      await timeout(50);
+
+      assert.dom('output').hasText('new');
+      assert.strictEqual(test.debouncedValue, test.value, `Value is "${test.value}" after ~205ms`);
+  });
+})