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

timer: ref/unref return self #2905

Closed
wants to merge 1 commit into from
Closed

timer: ref/unref return self #2905

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
4 changes: 4 additions & 0 deletions doc/api/timers.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,16 @@ In the case of `setTimeout` when you `unref` you create a separate timer that
will wakeup the event loop, creating too many of these may adversely effect
event loop performance -- use wisely.

Returns the timer.

## ref()

If you had previously `unref()`d a timer you can call `ref()` to explicitly
request the timer hold the program open. If the timer is already `ref`d calling
`ref` again will have no effect.

Returns the timer.

## setImmediate(callback[, arg][, ...])

To schedule the "immediate" execution of `callback` after I/O events
Expand Down
3 changes: 3 additions & 0 deletions lib/timers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,11 +330,13 @@ Timeout.prototype.unref = function() {
this._handle.domain = this.domain;
this._handle.unref();
}
return this;
};

Timeout.prototype.ref = function() {
if (this._handle)
this._handle.ref();
return this;
};

Timeout.prototype.close = function() {
Expand All @@ -345,6 +347,7 @@ Timeout.prototype.close = function() {
} else {
exports.unenroll(this);
}
return this;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW: Can we either document close or make it _close?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably can't rename it, maybe could document it, though I don't know if it has a use case. Isn't the timer API inherited from browsers?

Copy link
Contributor

Choose a reason for hiding this comment

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

Node's Timer API is pretty different from what browsers use. Browsers return numerical IDs while we return Timeout objects. I'd say if there's no public use case, we should move it to _close.

In the long run, I'd like to see us getting closer to the standardized WindowTimers which means returning numerical values, and possibly introduce helper methods for added functionality (Timer#ref for example).



Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-timers-unref.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ var interval_fired = false,
var LONG_TIME = 10 * 1000;
var SHORT_TIME = 100;

assert.doesNotThrow(function() {
setTimeout(function() {}, 10).unref().ref().unref();
}, 'ref and unref are chainable');

assert.doesNotThrow(function() {
setInterval(function() {}, 10).unref().ref().unref();
}, 'ref and unref are chainable');

setInterval(function() {
interval_fired = true;
}, LONG_TIME).unref();
Expand Down