You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have a question on this deprecation. I am using Timers.active to start a passive timer on a socket and Timers.unenroll to stop it. I see it says I need to change unenroll to clearTimeout but nothing about Timers.active. Will clearTimeout unenroll the timer created from Timers.active ?
It will work, yes, but you should probably move to using setTimeout. (The old API causes more issues than it solves perf-wise.)
Also, Timers.active doesn't really initialize everything properly. (That's what enroll() was for.)
Timers.active() is not yet deprecated as it is useful to refresh existing timeouts (even those made by setTimeout in-place without allocating new resources.
I could do that sooner I suppose, as the API seems fine to me. I guess browsers might also be interested in this so maybe in the distant future refreshTimeout() would be better, but I am not going to bother with trying to do web standards stuff.
Maybe. I'm fine with moving, just not sure how to make the move. This came out from the "mysql" module. Is there like a guide I can read to understand the changes I need to make?
Also I saw in the code there is Timeout.prototype[refreshFnSymbol] already. Any reason not to bring it out from behind a symbol in like 10.1? Seems odd to mix the two APIs temporarily. It would also be an easy way to feature detect from my question above: see if there is a refresh method for timers, right?
Is there a way to feature detect when to use this new method vs the old method?
Always. I think you need to be on like Node 0.8 for the full API to be realistically worse for perf. The APIs in the example exist and function correctly on all versions of Node.
Any reason not to bring it out from behind a symbol in like 10.1?
From the above, I wasn't sure if something like refreshTimeout(timer) would be a better API. Browsers could make use of something like that (although as stated I don't really want a part in that).
Such an approach could also work better with #19683. (Although something like a getTimerFromId would also work.)
Activity
jasnell commentedon Apr 24, 2018
/cc @Fishrock123
Fishrock123 commentedon Apr 24, 2018
It will work, yes, but you should probably move to using
setTimeout. (The old API causes more issues than it solves perf-wise.)Also,
Timers.activedoesn't really initialize everything properly. (That's whatenroll()was for.)Timers.active()is not yet deprecated as it is useful to refresh existing timeouts (even those made bysetTimeoutin-place without allocating new resources.In the future I want to expose this internal function which does that in a better way but I wasn't sure what the end API should be: https://github.com/nodejs/node/blob/master/lib/internal/timers.js#L85-L96
I could do that sooner I suppose, as the API seems fine to me. I guess browsers might also be interested in this so maybe in the distant future
refreshTimeout()would be better, but I am not going to bother with trying to do web standards stuff.Fishrock123 commentedon Apr 24, 2018
Maybe the deprecation should also mention that if you are using
Timers.activein that way you should probably move away from it...?dougwilson commentedon Apr 24, 2018
Maybe. I'm fine with moving, just not sure how to make the move. This came out from the "mysql" module. Is there like a guide I can read to understand the changes I need to make?
dougwilson commentedon Apr 24, 2018
I'm just basically enrolling an object to a timer and I extend the timeout every time a data chunk arrives so it is basically an inactivity timeout.
Fishrock123 commentedon Apr 24, 2018
This older code:
Becomes something like:
dougwilson commentedon Apr 24, 2018
Awesome, will try it out. Is there a way to feature detect when to use this new method vs the old method?
dougwilson commentedon Apr 24, 2018
Also I saw in the code there is Timeout.prototype[refreshFnSymbol] already. Any reason not to bring it out from behind a symbol in like 10.1? Seems odd to mix the two APIs temporarily. It would also be an easy way to feature detect from my question above: see if there is a refresh method for timers, right?
Fishrock123 commentedon Apr 25, 2018
Always. I think you need to be on like Node 0.8 for the full API to be realistically worse for perf. The APIs in the example exist and function correctly on all versions of Node.
From the above, I wasn't sure if something like
refreshTimeout(timer)would be a better API. Browsers could make use of something like that (although as stated I don't really want a part in that).Such an approach could also work better with #19683. (Although something like a
getTimerFromIdwould also work.)Ehhh, I'll open a PR to make it public. :)
Fishrock123 commentedon Apr 25, 2018
Ok, PR up at #20298
timers: make timer.refresh() a public API
timers: make timer.refresh() a public API
timers: make timer.refresh() a public API
Trott commentedon Nov 17, 2018
#20298 landed and was released in 11.0.0.
@dougwilson Can this issue be closed? Or is there more to be done here?
dougwilson commentedon Nov 17, 2018
Yes, all resolved, thank you.
apurv195 commentedon Nov 11, 2019
Just update the mysql library to latest version
npm install mysql@2.16.0 --save