Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 6 additions & 0 deletions doc/api/timers.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ of the Node.js application.

Returns a reference to the `Timeout`.

### timeout[Symbol.toPrimitive]()

When coercing a `Timeout` to a primitive, a primitive will be generated that can be used with the appropriate function that can be used to clear the `Timeout`. This allows enhanced compatibility with browser `setTimeout`, and `setInterval` implementations.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Please wrap at 80 characters :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wrap at 80 cols.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed


Returns a `number`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Put

* Returns: {integer}

right below the heading.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed


## Scheduling Timers

A timer in Node.js is an internal construct that calls a given function after
Expand Down
2 changes: 1 addition & 1 deletion lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ exports.setInterval = function(callback, repeat, arg1, arg2, arg3) {
return timeout;
};

exports.clearInterval = function(timer) {
const clearInterval = exports.clearInterval = function(timer) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why the addition of clearInterval variable? Doesn't seem used anywhere.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

was added in e32b969 as a bug fix to previous iteration

if (typeof timer === 'number' || typeof timer === 'string') {
if (timer in KNOWN_TIMERS) {
clearInterval(KNOWN_TIMERS[timer]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we may want to avoid accessing global.clearInterval here … maybe doing something like we do for clearTimeout, i.e. const clearInterval = exports.clearInterval = … is a good idea here too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ah yes, that would be a bug if that happened. I'm not 100% sure on why these functions are not named looking at them...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would timer = KNOWN_TIMERS[timer] work? This would avoid a recursion.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed

Expand Down