Update timers.js

[simonkcleung]

#7931
callback should run as if global.callback() in setTimout/setInterval

[rlidwka]

Binding to a timer object actually makes sense. This way for example you could close() an interval even when the reference to an interval is not saved:

setInterval(function() {
    if (some_condition) this.close(); // this would stop the timer
}, 1000);

This is much better idea for this than one used in the example in #7931.

And since setTimeout is intentionally different from browsers, there is no reason to keep browser semantics just for the sake of it. We don't support setTimeout("console.log('hi there')", 1), do we? Here is the same thing. :)

[simonkcleung]

It is not a matter of browser semantics.
In Nodejs Manual, it says:

setTimeout(cb, ms):
Run callback cb after at least ms milliseconds.
setInterval(cb, ms):
Run callback cb repeatedly every ms milliseconds.

So simply run the callback cb() is what the manual say.
However, we need to bind the callback to an object because we need to bind the arguments too. (the correct syntax of setTimeout should be setTimeout(cb, ms [,args...]))
To choose, I think the global object is better than the timer because global.cb() is nearly the same as cb().
Unless the manual clearly specify it, the current behavior is not correct.

[trevnorris]

As far as I see it, this proposal breaks backwards compatibility with code
that currently does the following:

setInterval(function() {
clearInterval(this);
}, 1000);

[vkurchatkin]

@trevnorris this code relies on undocumented behaviour, doesn't it?

[trevnorris]

@vkurchatkin True it's undocumented, but it follows the EventEmitter convention that is throughout Node. Also the module is Locked, so adding functionality like this.close() would need to be an unlikely exception.

Regardless, we're not allowing any use of Function#bind(). It's a performance hit that's completely unnecessary.

[simonkcleung]

For the convention issue, setTimeout/setInterval is a method of global object, the this should refer to the global object.
emitter.on('event',callback) this object in the callback is the emitter
global.setTimeout(callback,msec) why this object here in node is other thing?

close method in the Timeout object is undocumented. Also, it makes the api unnecessarily complicated. In functionality, it is a duplicate of clearTimeout function. Programmers should use clearTimeout function I think.

[trevnorris]

First, the close() thing is out.

Now, as far as switching setTimeout/setInterval to put this == global. It makes no sense in terms of functionality. As it is, setting the context to the same as the timer itself is useful and stateful. Simply making it the global (which you already have access to) just removes functionality.

Going to close this, since the functionality isn't going to change, but I am curious. Why would you want to change the method context to global when you already have access to it?