Repository: sinonjs/fake-timers
Pull request #425 (2022-03-02 07:05:08)

fix(timer.refresh): should just change `callAt`
#### Purpose (TL;DR) - mandatory

`timer.refresh` looks completely broken to me (and is definitely at least _somewhat_ broken, see https://github.com/facebook/jest/issues/12527).

1. It uses global `setTimeout` and `clearTimeout`, instead of the ones defined in the `global` passed in
1. it (tries to) remove the old timer and schedule a new one, "changing" its ID. This:
    1. Does not remove the existing one, as that's not an actual real timer
    1. Break identity since we "schedule" (actually, for real, schedules) a _new_ timer instead of refreshing the existing one (try `timer.refresh` in node - `Symbol`s `asyncId` and `triggerId` never changes, only `_idleStart`. also `t.refresh() === t`)

fatso83: if good, merge

Patch:
diff --git a/package-lock.json b/package-lock.json
index d5799a6..d06dd30 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -3575,6 +3575,7 @@
       "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-1.2.13.tgz",
       "integrity": "sha512-oWb1Z6mkHIskLzEJ/XWX0srkpkTQ7vaopMQkyaEIoq0fmtFVxOthb8cCxeT+p3ynTdkk/RZwbgG4brR5BeWECw==",
       "dev": true,
+      "hasInstallScript": true,
       "optional": true,
       "os": [
         "darwin"
diff --git a/src/fake-timers-src.js b/src/fake-timers-src.js
index ddcb245..8dcc78d 100644
--- a/src/fake-timers-src.js
+++ b/src/fake-timers-src.js
@@ -598,9 +598,14 @@ function withGlobal(_global) {
                     return this.refed;
                 },
                 refresh: function () {
-                    clearTimeout(timer.id);
-                    const args = [timer.func, timer.delay].concat(timer.args);
-                    return setTimeout.apply(null, args);
+                    timer.callAt =
+                        clock.now +
+                        (parseInt(timer.delay) || (clock.duringTick ? 1 : 0));
+
+                    // it _might_ have been removed, but if not the assignment is perfectly fine
+                    clock.timers[timer.id] = timer;
+
+                    return res;
                 },
                 [Symbol.toPrimitive]: function () {
                     return timer.id;


Test patch:
diff --git a/test/fake-timers-test.js b/test/fake-timers-test.js
index 2dd5fc3..c3a491c 100644
--- a/test/fake-timers-test.js
+++ b/test/fake-timers-test.js
@@ -3607,16 +3607,16 @@ describe("FakeTimers", function () {
             }
         });
 
-        it("global fake setTimeout().refresh() should return timer", function () {
+        it("global fake setTimeout().refresh() should return same timer", function () {
             this.clock = FakeTimers.install();
             const stub = sinon.stub();
 
             if (typeof setTimeout(NOOP, 0) === "object") {
-                const to = setTimeout(stub, 1000).refresh();
-                assert.isNumber(Number(to));
-                assert.isFunction(to.ref);
-                assert.isFunction(to.refresh);
+                const timeout = setTimeout(stub, 1000);
+                const to = timeout.refresh();
+                assert(timeout === to);
             }
+            this.clock.uninstall();
         });
 
         it("replaces global clearTimeout", function () {
@@ -4834,7 +4834,7 @@ describe("#368 - timeout.refresh setTimeout arguments", function () {
         }
     });
 
-    it("should forward  arguments passed to setTimeout", function () {
+    it("should forward arguments passed to setTimeout", function () {
         const clock = FakeTimers.install();
         const stub = sinon.stub();
 
@@ -4863,13 +4863,19 @@ describe("#187 - Support timeout.refresh in node environments", function () {
         clock.uninstall();
     });
 
-    it("assigns a new id to the refreshed timer", function () {
+    it("only calls stub once if not fired at time of refresh", function () {
         const clock = FakeTimers.install();
         const stub = sinon.stub();
+
         if (typeof setTimeout(NOOP, 0) === "object") {
             const t = setTimeout(stub, 1000);
-            const t2 = t.refresh();
-            refute.same(Number(t), Number(t2));
+            clock.tick(999);
+            assert(stub.notCalled);
+            t.refresh();
+            clock.tick(999);
+            assert(stub.notCalled);
+            clock.tick(1);
+            assert(stub.calledOnce);
         }
         clock.uninstall();
     });