timers: Implement conversion of timer argument

[starkwang]

"timer" argument should be converted to primitive, which makes the node API consistent with the browsers behavior.

Fix: #7792

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

timers

[refack]

CI: https://ci.nodejs.org/job/node-test-pull-request/9645/

[TimothyGu]

So this is quite an interesting case, since browsers' timer methods return a number instead of an object, like we do. As such the clearing methods also need to coerce the argument to a number. However, while it seems more consistent to add coercion to our clearing methods, it breaks the language pact that @@toPrimitive, valueOf and toString should always return a primitive. It's unfortunate that we are using an object here rather than a number, but because of that I'm strongly -1 against this change.

[refack]

You could merge the last two as timer == null. It's a lite idiom used quite often in "core" to mean "something nully".
Also BTW typeof undefined !== object`, so the second clause covers it.

[refack]

There is a case that need considering:

node/lib/net.js

Lines 381 to 396 in 472a665

	Socket.prototype.setTimeout = function(msecs, callback) {
	if (msecs === 0) {
	timers.unenroll(this);
	if (callback) {
	this.removeListener('timeout', callback);
	}
	} else {
	timers.enroll(this, msecs);
	timers._unrefActive(this);
	if (callback) {
	this.once('timeout', callback);
	}
	}
	return this;
	};

(this is something we want to refactor in the future Ref: #14209)

[refack]

So this is quite an interesting case, since browsers' timer methods return a number instead of an object, like we do. As such the clearing methods also need to coerce the argument to a number. However, while it seems more consistent to add coercion to our clearing methods, it breaks the language pact that @@toPrimitive, valueOf and toString should always return a primitive. It's unfortunate that we are using an object here rather than a number, but because of that I'm strongly -1 against this change.

What will be the ramification of changing node's setTimeout and setInterval to return a primitive? IMHO the docs are vague enough about the type returned that we could consider it - https://nodejs.org/dist/latest/docs/api/timers.html#timers_timers

[mscdex]

I'm -1 as well. This will introduce unnecessary overhead for no real benefit.

[Fishrock123]

The timeout instance APIs ref() and unref() make this impractical if not impossible.

I don't think this is worth the breakage and I'd consider it dead in the water as that ship sailed half a decade ago. Sorry. :/