Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

setInterval doesn't comply with DOM-version #8066

Closed
loyd opened this issue Aug 3, 2014 · 2 comments
Closed

setInterval doesn't comply with DOM-version #8066

loyd opened this issue Aug 3, 2014 · 2 comments
Labels

Comments

@loyd
Copy link

loyd commented Aug 3, 2014

Checked on x86_64, v0.10.30 and v0.11.14-pre, linux.

Firslty, setInterval determines delay between executions. For setInterval(sleep(20), 100):

------|====----------------|====----------------|====---
     0    20              120  140             240  260

DOM-version setInterval determines delay between runs:

------|====-----------|====-----------|====-----------|=
     0    20         100  120        200  220        300

Use the following snippet to check it out:

var prev = Date.now();
setInterval(function() {
  var s = Date.now();
  while(Date.now() - s < 20);
  var e = Date.now();
  console.log('Start', s, 'End', e, 'Delta', s - prev);
  prev = e;
}, 100);

Node v0.11.14-pre 'Delta': 102, 124, 119, 120, 120, ...
Node v0.10.30 'Delta': 101, 102, 100, 100, 100, ...
DOM 'Delta': 100, 80, 81, 80, 80, 81, ...

Secondly, overlay of calls:

var prev = Date.now();
setInterval(function() {
  var s = Date.now();
  while(Date.now() - s < 200);
  var e = Date.now();
  console.log('Start', s, 'End', e, 'Delta', s - prev);
  prev = e;
}, 100);

Node v0.11.14-pre 'Delta': 101, 302, 300, 300, 300, ...
Node v0.10.30 'Delta': 101, 101, 100, 100, ...
DOM 'Delta': 101, 0, 0, 1, 0, 1, 0, 0, ...

@loyd
Copy link
Author

loyd commented Aug 3, 2014

Well, the obvious solution of first is:

diff --git a/lib/timers.js b/lib/timers.js
index 3039b49..21d4003 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -284,8 +284,6 @@ exports.setInterval = function(callback, repeat) {
   return timer;

   function wrapper() {
-    callback.apply(this, args);
-    // If callback called clearInterval().
     if (timer._repeat === false) return;
     // If timer is unref'd (or was - it's permanently removed from the list.)
     if (this._handle) {
@@ -294,6 +292,8 @@ exports.setInterval = function(callback, repeat) {
       timer._idleTimeout = repeat;
       exports.active(timer);
     }
+
+    callback.apply(this, args);
   }
 };

@loyd loyd changed the title setInterval doesn't comply setInterval doesn't comply DOM-version Aug 3, 2014
@loyd
Copy link
Author

loyd commented Aug 3, 2014

Why should we implement interval over "idle timeout" pattern as timeouts?

@loyd loyd changed the title setInterval doesn't comply DOM-version setInterval doesn't comply with DOM-version Aug 3, 2014
@loyd loyd mentioned this issue Aug 9, 2014
@trevnorris trevnorris self-assigned this Jan 22, 2015
@trevnorris trevnorris removed their assignment Feb 18, 2016
@Trott Trott closed this as completed Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants