-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
this binding incorrect for setTimeouf(..) and setInterval(..) #7931
Description
In browsers, this code (however inadvisable it may be, it's still valid and spec correct) behaves as expected, which is that the interval is stopped after 5 seconds of printing "Hello World":
intv = setInterval(function(){
console.log("Hello World");
},1000);
// elsewhere in the code base
setTimeout(function() {
clearInterval(this.intv);
},5000);But if you try the same code in node (0.10.29), the interval is never cleared, so the "Hello World" printing keeps going forever.
The problem comes down to the this binding in the underlying function calls.
According to the HTML5 spec, 6.4: Timers, and specifically 6.4.1: timer initialization steps, the this binding should be the window object (technically the "window-proxy") for normal apps, or the worker-global-scope in the case of web workers.
One could take that as a pretty strong precedent that the this binding should always be the appropriate hosting environment's global. But in node.js, it's not, it's some other object that's apparently related to the timer instance itself.
I have some real-world code using timers that works fine in a browser but is breaking when I port it to node, and it turns out this this binding issue is the direct cause.
The above code can be changed (without rearchitecting to use IIFE/module closures, etc), but it's uglier and less desirable:
intv = setInterval(function(){
console.log("Hello World");
},1000);
// elsewhere in the code base
setTimeout(function() {
var g = typeof window !== "undefined" ? window : typeof global !== "undefined" ? global : {};
clearInterval(g.intv);
},5000);
[Edit: I'm striking out the above note/code because it detracts my overall point and confuses the issue. sorry about that.]
I know the behavior of setTimeout(..) is controlled by HTML spec, not ES, and I know node isn't HTML, but with respect to certain on-top-of-or-related-to-ES touch points, the HTML spec takes a lot of care in doing sensible things, such as an explicit predictable this binding here, and it's frustrating that node.js diverges here.
Can we consider changing this behavior to follow the spec precedent?