Repository: sinonjs/fake-timers
Pull request #407 (2021-10-25 19:08:59)

Add native timer clean-up behavior
#### Purpose (TL;DR) - mandatory
See #406 for context.

Adds `shouldHandleNativeTimers` flag to the config (set to false by default). When enabled, calls to `clear/cancel` are forwarded to their real functions if the timer was not created by `FakeTimers`. 
<!--
> give a concise (one or two short sentences) description of what problem is being solved by this PR
>
> Example: Fix issue #123456 by re-structuring the colour selection conditional in method `paintBlue`
-->

<!--
#### Background (Problem in detail)  - optional
-->
<!--
> When relevant, give a more thorough description of what the problem the PR is trying to solve. Examples of good topics for this section are:
> * Link to an existing GitHub issue describing the problem
> * Describing the problem in greater detail than the TL;DR section above
> * How you discovered the issue if it's not already described as an issue on GitHub
> * Discussion of different approaches to solving this problem and why you chose your proposed solution
-->

<!--
#### Solution  - optional
-->
<!--
> When contributing code (and not just fixing typos, documentation and configuration), please describe why/how your solution works. This helps reviewers spot any mistakes in the implementation.
>
> Example:
> "This solution works by adding a `paintBlue()` method"
> Then your reviewer might spot a mistake in the implementation, if `paintBlue()` uses the colour red.
-->



Patch:
diff --git a/README.md b/README.md
index d5404f3..a96a65d 100644
--- a/README.md
+++ b/README.md
@@ -150,13 +150,14 @@ The `loopLimit` argument sets the maximum number of timers that will be run when
 
 Installs FakeTimers using the specified config (otherwise with epoch `0` on the global scope). The following configuration options are available
 
-| Parameter                  | Type        | Default                                                                                                                                                                                                         | Description                                                                                                                                                                                                                                       |
-| -------------------------- | ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `config.now`               | Number/Date | 0                                                                                                                                                                                                               | installs FakeTimers with the specified unix epoch                                                                                                                                                                                                 |
-| `config.toFake`            | String[]    | ["setTimeout", "clearTimeout", "setImmediate", "clearImmediate","setInterval", "clearInterval", "Date", "requestAnimationFrame", "cancelAnimationFrame", "requestIdleCallback", "cancelIdleCallback", "hrtime"] | an array with explicit function names to hijack. _When not set, FakeTimers will automatically fake all methods **except** `nextTick`_ e.g., `FakeTimers.install({ toFake: ["setTimeout","nextTick"]})` will fake only `setTimeout` and `nextTick` |
-| `config.loopLimit`         | Number      | 1000                                                                                                                                                                                                            | the maximum number of timers that will be run when calling runAll()                                                                                                                                                                               |
-| `config.shouldAdvanceTime` | Boolean     | false                                                                                                                                                                                                           | tells FakeTimers to increment mocked time automatically based on the real system time shift (e.g. the mocked time will be incremented by 20ms for every 20ms change in the real system time)                                                      |
-| `config.advanceTimeDelta`  | Number      | 20                                                                                                                                                                                                              | relevant only when using with `shouldAdvanceTime: true`. increment mocked time by `advanceTimeDelta` ms every `advanceTimeDelta` ms change in the real system time.                                                                               |
+| Parameter                        | Type        | Default                                                                                                                                                                                                         | Description                                                                                                                                                                                                                                       |
+| -------------------------------- | ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `config.now`                     | Number/Date | 0                                                                                                                                                                                                               | installs FakeTimers with the specified unix epoch                                                                                                                                                                                                 |
+| `config.toFake`                  | String[]    | ["setTimeout", "clearTimeout", "setImmediate", "clearImmediate","setInterval", "clearInterval", "Date", "requestAnimationFrame", "cancelAnimationFrame", "requestIdleCallback", "cancelIdleCallback", "hrtime"] | an array with explicit function names to hijack. _When not set, FakeTimers will automatically fake all methods **except** `nextTick`_ e.g., `FakeTimers.install({ toFake: ["setTimeout","nextTick"]})` will fake only `setTimeout` and `nextTick` |
+| `config.loopLimit`               | Number      | 1000                                                                                                                                                                                                            | the maximum number of timers that will be run when calling runAll()                                                                                                                                                                               |
+| `config.shouldAdvanceTime`       | Boolean     | false                                                                                                                                                                                                           | tells FakeTimers to increment mocked time automatically based on the real system time shift (e.g. the mocked time will be incremented by 20ms for every 20ms change in the real system time)                                                      |
+| `config.advanceTimeDelta`        | Number      | 20                                                                                                                                                                                                              | relevant only when using with `shouldAdvanceTime: true`. increment mocked time by `advanceTimeDelta` ms every `advanceTimeDelta` ms change in the real system time.                                                                               |
+| `config.shouldClearNativeTimers` | Boolean     | false                                                                                                                                                                                                           | tells FakeTimers to clear 'native' (i.e. not fake) timers by delegating to their respective handlers. These are not cleared by default, leading to potentially unexpected behavior if timers existed prior to installing FakeTimers.              |
 
 ### `var id = clock.setTimeout(callback, timeout)`
 
diff --git a/src/fake-timers-src.js b/src/fake-timers-src.js
index d46ead5..e1a0a1e 100644
--- a/src/fake-timers-src.js
+++ b/src/fake-timers-src.js
@@ -83,6 +83,7 @@ const globalObject = require("@sinonjs/commons").global;
  * @property {function(number[]): number[]} hrtime - process.hrtime (legacy)
  * @property {function(): void} uninstall Uninstall the clock.
  * @property {Function[]} methods - the methods that are faked
+ * @property {boolean} [shouldClearNativeTimers] inherited from config
  */
 /* eslint-enable jsdoc/require-property-description */
 
@@ -95,6 +96,7 @@ const globalObject = require("@sinonjs/commons").global;
  * @property {number} [loopLimit] the maximum number of timers that will be run when calling runAll()
  * @property {boolean} [shouldAdvanceTime] tells FakeTimers to increment mocked time automatically (default false)
  * @property {number} [advanceTimeDelta] increment mocked time every <<advanceTimeDelta>> ms (default: 20ms)
+ * @property {boolean} [shouldClearNativeTimers] forwards clear timer calls to native functions if they are not fakes (default: false)
  */
 
 /* eslint-disable jsdoc/require-property-description */
@@ -134,6 +136,7 @@ function withGlobal(_global) {
     const userAgent = _global.navigator && _global.navigator.userAgent;
     const isRunningInIE = userAgent && userAgent.indexOf("MSIE ") > -1;
     const maxTimeout = Math.pow(2, 31) - 1; //see https://heycam.github.io/webidl/#abstract-opdef-converttoint
+    const idCounterStart = 1e12; // arbitrarily large number to avoid collisions with native timer IDs
     const NOOP = function () {
         return undefined;
     };
@@ -192,7 +195,7 @@ function withGlobal(_global) {
     _global.clearTimeout(timeoutResult);
 
     const NativeDate = _global.Date;
-    let uniqueTimerId = 1;
+    let uniqueTimerId = idCounterStart;
 
     /**
      * @param {number} num
@@ -560,6 +563,11 @@ function withGlobal(_global) {
             timer.animation = true;
         }
 
+        if (timer.hasOwnProperty("idleCallback")) {
+            timer.type = "IdleCallback";
+            timer.idleCallback = true;
+        }
+
         if (!clock.timers) {
             clock.timers = {};
         }
@@ -573,7 +581,6 @@ function withGlobal(_global) {
 
         if (addTimerReturnsObject) {
             const res = {
-                id: timer.id,
                 ref: function () {
                     return res;
                 },
@@ -585,6 +592,9 @@ function withGlobal(_global) {
                     const args = [timer.func, timer.delay].concat(timer.args);
                     return setTimeout.apply(null, args);
                 },
+                [Symbol.toPrimitive]: function () {
+                    return timer.id;
+                },
             };
             return res;
         }
@@ -726,6 +736,40 @@ function withGlobal(_global) {
         }
     }
 
+    /**
+     * Gets clear handler name for a given timer type
+     * @param {string} ttype
+     */
+    function getClearHandler(ttype) {
+        if (ttype === "IdleCallback" || ttype === "AnimationFrame") {
+            return `cancel${ttype}`;
+        }
+        return `clear${ttype}`;
+    }
+
+    /**
+     * Gets schedule handler name for a given timer type
+     * @param {string} ttype
+     */
+    function getScheduleHandler(ttype) {
+        if (ttype === "IdleCallback" || ttype === "AnimationFrame") {
+            return `request${ttype}`;
+        }
+        return `set${ttype}`;
+    }
+
+    /**
+     * Creates an anonymous function to warn only once
+     */
+    function createWarnOnce() {
+        let calls = 0;
+        return function (msg) {
+            // eslint-disable-next-line
+            !calls++ && console.warn(msg);
+        };
+    }
+    const warnOnce = createWarnOnce();
+
     /**
      * @param {Clock} clock
      * @param {number} timerId
@@ -742,9 +786,24 @@ function withGlobal(_global) {
             clock.timers = {};
         }
 
-        // in Node, timerId is an object with .ref()/.unref(), and
-        // its .id field is the actual timer id.
-        const id = typeof timerId === "object" ? timerId.id : timerId;
+        // in Node, the ID is stored as the primitive value for `Timeout` objects
+        // for `Immediate` objects, no ID exists, so it gets coerced to NaN
+        const id = Number(timerId);
+
+        if (Number.isNaN(id) || id < idCounterStart) {
+            const handlerName = getClearHandler(ttype);
+
+            if (clock.shouldClearNativeTimers === true) {
+                const nativeHandler = clock[`_${handlerName}`];
+                return typeof nativeHandler === "function"
+                    ? nativeHandler(timerId)
+                    : undefined;
+            }
+            warnOnce(
+                `FakeTimers: ${handlerName} was invoked to clear a native timer instead of one created by this library.` +
+                    "\nTo automatically clean-up native timers, use `shouldClearNativeTimers`."
+            );
+        }
 
         if (clock.timers.hasOwnProperty(id)) {
             // check that the ID matches a timer of the correct type
@@ -756,14 +815,8 @@ function withGlobal(_global) {
             ) {
                 delete clock.timers[id];
             } else {
-                const clear =
-                    ttype === "AnimationFrame"
-                        ? "cancelAnimationFrame"
-                        : `clear${ttype}`;
-                const schedule =
-                    timer.type === "AnimationFrame"
-                        ? "requestAnimationFrame"
-                        : `set${timer.type}`;
+                const clear = getClearHandler(ttype);
+                const schedule = getScheduleHandler(timer.type);
                 throw new Error(
                     `Cannot clear timer: timer created with ${schedule}() but cleared with ${clear}()`
                 );
@@ -1050,13 +1103,14 @@ function withGlobal(_global) {
                     typeof timeout === "undefined"
                         ? timeToNextIdlePeriod
                         : Math.min(timeout, timeToNextIdlePeriod),
+                idleCallback: true,
             });
 
-            return result.id || result;
+            return Number(result);
         };
 
         clock.cancelIdleCallback = function cancelIdleCallback(timerId) {
-            return clearTimer(clock, timerId, "Timeout");
+            return clearTimer(clock, timerId, "IdleCallback");
         };
 
         clock.setTimeout = function setTimeout(func, timeout) {
@@ -1158,7 +1212,7 @@ function withGlobal(_global) {
                 animation: true,
             });
 
-            return result.id || result;
+            return Number(result);
         };
 
         clock.cancelAnimationFrame = function cancelAnimationFrame(timerId) {
@@ -1582,6 +1636,8 @@ function withGlobal(_global) {
         config = typeof config !== "undefined" ? config : {};
         config.shouldAdvanceTime = config.shouldAdvanceTime || false;
         config.advanceTimeDelta = config.advanceTimeDelta || 20;
+        config.shouldClearNativeTimers =
+            config.shouldClearNativeTimers || false;
 
         if (config.target) {
             throw new TypeError(
@@ -1591,6 +1647,7 @@ function withGlobal(_global) {
 
         let i, l;
         const clock = createClock(config.now, config.loopLimit);
+        clock.shouldClearNativeTimers = config.shouldClearNativeTimers;
 
         clock.uninstall = function () {
             return uninstall(clock, config);


Test patch:
diff --git a/test/fake-timers-test.js b/test/fake-timers-test.js
index 200d3d1..673e8de 100644
--- a/test/fake-timers-test.js
+++ b/test/fake-timers-test.js
@@ -272,7 +272,7 @@ describe("FakeTimers", function () {
             const result = this.clock.setTimeout(function () {}, 10);
 
             if (typeof result === "object") {
-                assert.isNumber(result.id);
+                assert.isNumber(Number(result));
             } else {
                 assert.isNumber(result);
             }
@@ -285,6 +285,12 @@ describe("FakeTimers", function () {
             refute.equals(id2, id1);
         });
 
+        it("starts id from a large number", function () {
+            const timer = this.clock.setTimeout(function () {}, 10);
+
+            assert.isTrue(Number(timer) >= 1e12);
+        });
+
         it("sets timers on instance", function () {
             const clock1 = FakeTimers.createClock();
             const clock2 = FakeTimers.createClock();
@@ -552,7 +558,7 @@ describe("FakeTimers", function () {
             const result = this.clock.setImmediate(NOOP);
 
             if (typeof result === "object") {
-                assert.isNumber(result.id);
+                assert.isNumber(Number(result));
             } else {
                 assert.isNumber(result);
             }
@@ -3156,7 +3162,7 @@ describe("FakeTimers", function () {
             const result = this.clock.setInterval(function () {}, 10);
 
             if (typeof result === "object") {
-                assert.isNumber(result.id);
+                assert.isNumber(Number(result));
             } else {
                 assert.isNumber(result);
             }
@@ -3565,7 +3571,7 @@ describe("FakeTimers", function () {
             const to = setTimeout(stub, 1000);
 
             if (typeof setTimeout(NOOP, 0) === "object") {
-                assert.isNumber(to.id);
+                assert.isNumber(Number(to));
                 assert.isFunction(to.ref);
                 assert.isFunction(to.unref);
             } else {
@@ -3579,7 +3585,7 @@ describe("FakeTimers", function () {
 
             if (typeof setTimeout(NOOP, 0) === "object") {
                 const to = setTimeout(stub, 1000).ref();
-                assert.isNumber(to.id);
+                assert.isNumber(Number(to));
                 assert.isFunction(to.ref);
                 assert.isFunction(to.unref);
             }
@@ -3591,7 +3597,7 @@ describe("FakeTimers", function () {
 
             if (typeof setTimeout(NOOP, 0) === "object") {
                 const to = setTimeout(stub, 1000).unref();
-                assert.isNumber(to.id);
+                assert.isNumber(Number(to));
                 assert.isFunction(to.ref);
                 assert.isFunction(to.unref);
             }
@@ -3603,7 +3609,7 @@ describe("FakeTimers", function () {
 
             if (typeof setTimeout(NOOP, 0) === "object") {
                 const to = setTimeout(stub, 1000).refresh();
-                assert.isNumber(to.id);
+                assert.isNumber(Number(to));
                 assert.isFunction(to.ref);
                 assert.isFunction(to.refresh);
             }
@@ -3975,6 +3981,92 @@ describe("FakeTimers", function () {
         });
     });
 
+    describe("shouldClearNativeTimers", function () {
+        function createCallback(done, succeed) {
+            return function () {
+                if (succeed) {
+                    done();
+                } else {
+                    done(new Error("Timer was not cleared."));
+                }
+            };
+        }
+
+        afterEach(function () {
+            if (this.clock) {
+                this.clock.uninstall();
+            }
+        });
+
+        it("outputs a warning once if not enabled", function (done) {
+            const timer = globalObject.setTimeout(createCallback(done, true));
+            const stub = sinon.stub(globalObject.console, "warn");
+
+            this.clock = FakeTimers.install();
+            globalObject.clearTimeout(timer);
+            globalObject.clearTimeout(timer);
+            assert.equals(stub.callCount, 1);
+        });
+
+        it("can clear setTimeout", function (done) {
+            const timer = globalObject.setTimeout(createCallback(done, false));
+            globalObject.setTimeout(createCallback(done, true));
+
+            this.clock = FakeTimers.install({ shouldClearNativeTimers: true });
+            globalObject.clearTimeout(timer);
+        });
+
+        it("can clear setInterval", function (done) {
+            const timer = globalObject.setInterval(createCallback(done, false));
+            if (timer && typeof timer === "object") {
+                timer.unref(); // prevents hung failed test for node
+            }
+
+            globalObject.setTimeout(createCallback(done, true));
+            this.clock = FakeTimers.install({ shouldClearNativeTimers: true });
+            globalObject.clearInterval(timer);
+        });
+
+        it("can clear setImmediate", function (done) {
+            if (globalObject.setImmediate === undefined) {
+                this.skip();
+            }
+
+            const timer = globalObject.setImmediate(
+                createCallback(done, false)
+            );
+            globalObject.setImmediate(createCallback(done, true));
+            this.clock = FakeTimers.install({ shouldClearNativeTimers: true });
+            globalObject.clearImmediate(timer);
+        });
+
+        it("can clear requestAnimationFrame", function (done) {
+            if (globalObject.requestAnimationFrame === undefined) {
+                this.skip();
+            }
+
+            const timer = globalObject.requestAnimationFrame(
+                createCallback(done, false)
+            );
+            globalObject.requestAnimationFrame(createCallback(done, true));
+            this.clock = FakeTimers.install({ shouldClearNativeTimers: true });
+            globalObject.cancelAnimationFrame(timer);
+        });
+
+        it("can clear requestIdleCallback", function (done) {
+            if (globalObject.requestIdleCallback === undefined) {
+                this.skip();
+            }
+
+            const timer = globalObject.requestIdleCallback(
+                createCallback(done, false)
+            );
+            globalObject.requestIdleCallback(createCallback(done, true));
+            this.clock = FakeTimers.install({ shouldClearNativeTimers: true });
+            globalObject.cancelIdleCallback(timer);
+        });
+    });
+
     describe("requestAnimationFrame", function () {
         beforeEach(function () {
             this.clock = FakeTimers.createClock();
@@ -4729,7 +4821,7 @@ describe("#187 - Support timeout.refresh in node environments", function () {
         if (typeof setTimeout(NOOP, 0) === "object") {
             const t = setTimeout(stub, 1000);
             const t2 = t.refresh();
-            refute.same(t.id, t2.id);
+            refute.same(Number(t), Number(t2));
         }
         clock.uninstall();
     });
@@ -4918,7 +5010,7 @@ describe("loop limit stack trace", function () {
                     assert.equals(err.message, expectedMessage);
                     assert.equals(
                         new RegExp(
-                            `Error: ${expectedMessage}\\s+Timeout - recursiveCreateTimerTimeout\\s+(at )*recursiveCreateTimer`
+                            `Error: ${expectedMessage}\\s+IdleCallback - recursiveCreateTimerTimeout\\s+(at )*recursiveCreateTimer`
                         ).test(err.stack),
                         true
                     );
@@ -4935,7 +5027,7 @@ describe("loop limit stack trace", function () {
                 assert.equals(err.message, expectedMessage);
                 assert.equals(
                     new RegExp(
-                        `Error: ${expectedMessage}\\s+Timeout - recursiveCreateTimerTimeout\\s+(at )*recursiveCreateTimer`
+                        `Error: ${expectedMessage}\\s+IdleCallback - recursiveCreateTimerTimeout\\s+(at )*recursiveCreateTimer`
                     ).test(err.stack),
                     true
                 );