Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

timers: deprecate active() and _unrefActive() #26760

Closed
wants to merge 1 commit into from
Closed

timers: deprecate active() and _unrefActive() #26760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions doc/api/deprecations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2368,6 +2368,38 @@ Type: Runtime

The `_stream_wrap` module is deprecated.

<a id="DEP0126"></a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be DEP0XXX till landing (here, below and in the next section)?

Copy link
Contributor Author

@Fishrock123 Fishrock123 Mar 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really matter? ¯\_(ツ)_/¯

So long as it's updated if necessary at landing...

### DEP0126: timers.active()
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/26760
description: Runtime deprecation.
-->

Type: Runtime

The previously undocumented `timers.active()` is deprecated.
Please use the publicly documented [`timeout.refresh()`][] instead.
If re-referencing the timeout is necessary, [`timeout.ref()`][] can be used
with no performance impact since Node.js 10.

<a id="DEP0127"></a>
### DEP0127: timers._unrefActive()
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/26760
description: Runtime deprecation.
-->

Type: Runtime

The previously undocumented and "private" `timers._unrefActive()` is deprecated.
Please use the publicly documented [`timeout.refresh()`][] instead.
Copy link
Contributor

@mscdex mscdex Mar 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does timers._unrefActive() really do the same thing as timeout.refresh()? Shouldn't we only be recommending timeout.unref() as a replacement?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an awkward combination of both. It is essentially "refresh this timer as unrefed". Active is the same, but refed.

Perhaps there was an extra meaning back when {un}ref() made separate libuv handles. I'm not sure but that doesn't exist anymore regardless.

If unreferencing the timeout is necessary, [`timeout.unref()`][] can be used
with no performance impact since Node.js 10.

[`--pending-deprecation`]: cli.html#cli_pending_deprecation
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
[`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
Expand Down Expand Up @@ -2423,6 +2455,9 @@ The `_stream_wrap` module is deprecated.
[`script.createCachedData()`]: vm.html#vm_script_createcacheddata
[`setInterval()`]: timers.html#timers_setinterval_callback_delay_args
[`setTimeout()`]: timers.html#timers_settimeout_callback_delay_args
[`timeout.ref()`]: timers.html#timers_timeout_ref
[`timeout.refresh()`]: timers.html#timers_timeout_refresh
[`timeout.unref()`]: timers.html#timers_timeout_unref
[`tls.CryptoStream`]: tls.html#tls_class_cryptostream
[`tls.SecureContext`]: tls.html#tls_tls_createsecurecontext_options
[`tls.SecurePair`]: tls.html#tls_class_securepair
Expand Down
11 changes: 9 additions & 2 deletions lib/timers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,14 +376,21 @@ function clearImmediate(immediate) {
}

module.exports = {
_unrefActive: unrefActive,
active,
setTimeout,
clearTimeout,
setImmediate,
clearImmediate,
setInterval,
clearInterval,
_unrefActive: deprecate(
unrefActive,
'timers._unrefActive() is deprecated.' +
' Please use timeout.refresh() instead.',
'DEP0127'),
active: deprecate(
active,
'timers.active() is deprecated. Please use timeout.refresh() instead.',
'DEP0126'),
unenroll: deprecate(
unenroll,
'timers.unenroll() is deprecated. Please use clearTimeout instead.',
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-timers-max-duration-warning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ process.on('warning', common.mustCall((warning) => {
assert.strictEqual(lines[0], `${OVERFLOW} does not fit into a 32-bit signed` +
' integer.');
assert.strictEqual(lines.length, 2);
}, 5));
}, 6));


{
Expand Down