Merge pull request #45 from nfl/issue/44

(refactor) throttle foldCheck instead of debounce

Author: potench

examples/apps/infinite-scrolling/app.js

@@ -1,7 +1,7 @@
 /* eslint-disable react/sort-comp */
 import React, {Component} from "react";
 import Radium from "radium";
-import debounce from "debounce";
+import debounce from "throttle-debounce/debounce";
 import {Bling as Gpt, Events} from "react-gpt"; // eslint-disable-line import/no-unresolved
 import "../log";
 import Content from "./content";
@@ -40,14 +40,14 @@ class App extends Component {
         window.removeEventListener("resize", this.onScroll);
         this.stopTimer();
     }
-    onScroll = debounce(() => {
+    onScroll = debounce(66, () => {
         const scrollTop = window.scrollY || document.documentElement.scrollTop;
         if (scrollTop + window.innerHeight >= document.body.clientHeight) {
             this.setState({
                 page: ++this.state.page
             });
         }
-    }, 66)
+    })
     startTimer() {
         this.stopTimer();
         this.timer = setInterval(() => {

package.json

@@ -32,12 +32,11 @@
     "lib"
   ],
   "dependencies": {
-    "bundlesize": "^0.14.4",
-    "debounce": "^1.0.0",
     "deep-equal": "^1.0.1",
     "eventemitter3": "^2.0.2",
     "fbjs": "^0.8.1",
-    "hoist-non-react-statics": "^1.0.5"
+    "hoist-non-react-statics": "^1.0.5",
+    "throttle-debounce": "^1.0.1"
   },
   "devDependencies": {
     "babel-cli": "^6.5.1",
@@ -50,6 +49,7 @@
     "babel-preset-react": "^6.5.0",
     "babel-preset-stage-0": "^6.5.0",
     "babel-register": "^6.7.2",
+    "bundlesize": "^0.14.4",
     "chai": "^3.4.1",
     "codecov.io": "^0.1.6",
     "commitizen": "^2.8.1",

src/createManager.js

@@ -1,5 +1,5 @@
 import EventEmitter from "eventemitter3";
-import debounce from "debounce";
+import {debounce, throttle} from "throttle-debounce";
 import invariant from "fbjs/lib/invariant";
 import {canUseDOM} from "fbjs/lib/ExecutionEnvironment";
 import Events from "./Events";
@@ -131,14 +131,22 @@ export class AdManager extends EventEmitter {
         });
     }
 
-    _foldCheck = debounce(event => {
+    _foldCheck = throttle(20, event => {
         const instances = this.getMountedInstances();
         instances.forEach(instance => {
             if (instance.getRenderWhenViewable()) {
                 instance.foldCheck(event);
             }
         });
-    }, 66)
+
+        if (this.testMode) {
+            this._getTimer();
+        }
+    })
+
+    _getTimer() {
+        return Date.now();
+    }
 
     _handleMediaQueryChange = event => {
         if (this._syncCorrelator) {
@@ -307,7 +315,7 @@ export class AdManager extends EventEmitter {
         return true;
     }
 
-    render = debounce(() => {
+    render = debounce(4, () => {
         if (!this._initialRender) {
             return;
         }
@@ -375,7 +383,7 @@ export class AdManager extends EventEmitter {
 
             this._initialRender = false;
         });
-    }, 4)
+    })
 
     /**
      * Re-render(not refresh) all the ads in the page and the first ad will update the correlator value.
@@ -384,7 +392,7 @@ export class AdManager extends EventEmitter {
      * @method renderAll
      * @static
      */
-    renderAll = debounce(() => {
+    renderAll = debounce(4, () => {
         if (!this.apiReady) {
             return false;
         }
@@ -399,7 +407,7 @@ export class AdManager extends EventEmitter {
         });
 
         return true;
-    }, 4)
+    })
 
     getGPTVersion() {
         if (!this.apiReady) {

test/createManager.spec.js

@@ -399,7 +399,7 @@ describe("createManager", () => {
         adManager.render();
     });
 
-    it("debounces foldCheck", (done) => {
+    it("throttles foldCheck", (done) => {
         const instance = {
             props: {
                 sizeMapping: [{viewport: [0, 0], slot: [320, 50]}]
@@ -422,23 +422,45 @@ describe("createManager", () => {
 
         const foldCheck = sinon.stub(instance, "foldCheck");
         const foldCheck2 = sinon.stub(instance2, "foldCheck");
+        const getRenderWhenViewable = sinon.spy(instance, "getRenderWhenViewable");
+        const getRenderWhenViewable2 = sinon.spy(instance2, "getRenderWhenViewable");
+        const managerFoldCheck = sinon.spy(adManager, "_foldCheck");
+        const timer = sinon.spy(adManager, "_getTimer");
 
         adManager.addInstance(instance);
         adManager.addInstance(instance2);
 
+        const start = Date.now();
+        adManager._foldCheck();
+        adManager._foldCheck();
+        setTimeout(() => {
+            adManager._foldCheck();
+        }, 5);
+        setTimeout(() => {
+            adManager._foldCheck();
+        }, 10);
+        setTimeout(() => {
+            adManager._foldCheck();
+        }, 15);
+
         setTimeout(() => {
-            expect(foldCheck.calledOnce).to.be.true;
-            expect(foldCheck2.calledOnce).to.be.false;
+            expect(managerFoldCheck.callCount).to.equal(5);
+            expect(timer.calledTwice).to.be.true;
+            expect(timer.returnValues[1] - timer.returnValues[0]).to.be.above(19); // timer above 20ms timeout
+            expect(timer.returnValues[0] - start).to.be.below(5); // should start ~immediately
+            expect(foldCheck.calledTwice).to.be.true;
+            expect(foldCheck2.notCalled).to.be.true;
+
             foldCheck.restore();
             foldCheck2.restore();
+            getRenderWhenViewable.restore();
+            getRenderWhenViewable2.restore();
+            managerFoldCheck.restore();
+            timer.restore();
             adManager.removeInstance(instance);
             adManager.removeInstance(instance2);
             done();
         }, 100);
-
-        adManager._foldCheck();
-        adManager._foldCheck();
-        adManager._foldCheck();
     });
 
     it("renders all ads", (done) => {