sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
806 stars 107 forks source link

Error: Cannot clear timer: timer created with setInterval() but cleared with clearTimeout() #304

Closed slightlytwisted closed 4 years ago

slightlytwisted commented 4 years ago

fake-timers should not assert that calls to clearTimer() be passed only timers created by createTimer() and that calls to clearInterval() be passed only timers created by createInterval() because the HTML Living Standard indicates that:

Because clearTimeout() and clearInterval() clear entries from the same list, either method can be used to clear timers created by setTimeout() or setInterval().

The assertion in fake-timer should probably be relaxed.

fatso83 commented 4 years ago

Love a bug report with a WHATWG link to the reference ❤️

itayperry commented 4 years ago

Hey everyone, I'm taking a look at this :)

itayperry commented 4 years ago

A question :) This clearTimer() function appears in 2 different files, why is that? I need to change it, should I change it in both files? I would love for some help.

It seems like only the fake-timers-src.js affects this process:

fake-timers-issue-304

benjamingr commented 4 years ago

@itayperry fake-timers is generated from fake-timers-src through bundling it: https://github.com/sinonjs/fake-timers/blob/7f8322effa73a94dc43896cdfb49d63e821b32d1/package.json#L24 - you should generally only need to edit fake-timers-src.

itayperry commented 4 years ago

All ready and tested :) I hope this solves the issue properly.

324

benjamingr commented 4 years ago

Fixed in https://github.com/sinonjs/fake-timers/pull/324/commits/1f83b99639f57a3980847da7aaddcf3d11308b5d

NearHuscarl commented 4 years ago

In the mean time, I've got my test working again by patching the code from the commit mentioned above using patch-package. Here are the instructions for anyone wants to do the same

npx patch-package @sinonjs/fake-timers

After that, there should be a new file at patches/@sinonjs+fake-timers+6.0.1.patch with the following content

diff --git a/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js b/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js
index f723db5..7cdd059 100644
--- a/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js
+++ b/node_modules/@sinonjs/fake-timers/src/fake-timers-src.js
@@ -444,7 +444,11 @@ function withGlobal(_global) {
         if (clock.timers.hasOwnProperty(id)) {
             // check that the ID matches a timer of the correct type
             var timer = clock.timers[id];
-            if (timer.type === ttype) {
+            if (
+              timer.type === ttype ||
+              (timer.type === "Timeout" && ttype === "Interval") ||
+              (timer.type === "Interval" && ttype === "Timeout")
+            ) {
                 delete clock.timers[id];
             } else {
                 var clear =
tapz commented 3 years ago

How do I disable this functionality completely? I just want to mock dates and now face timers give an error with node pg, which is not my code. So I need to use something else than sinonjs because it has this kind of side effects.

fatso83 commented 3 years ago

@tapz Are you running 7.0.1? Cause this was released yesterday and includes the fix for this issue. Not sure how it applies to your case, as I have no idea what you are talking about (way too inspecific), but I would try to npm install @sinonjs/fake-timers@latest.

benjamingr commented 3 years ago

@tapz you can .install with a parameter indicating what to fake - and you can just not mock timers and only mock date.