timers: allow timers to be used as primitives

lundibundi · addaleax · commit 987e0cb7852e · 2020-09-22T23:48:06.000+02:00
This allows timers to be matched to numeric Ids and therefore used as keys of an Object, passed and stored without storing the Timer instance. clearTimeout/clearInterval is modified to support numeric/string Ids. Co-authored-by: Bradley Farias <bradley.meck@gmail.com> Co-authored-by: Anatoli Papirovski <apapirovski@mac.com> Refs: #21152 Backport-PR-URL: #34482 PR-URL: #34017 Backport-PR-URL: #34482 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Signed-off-by: Denys Otrishko <shishugi@gmail.com>
diff --git a/doc/api/timers.md b/doc/api/timers.md
@@ -125,6 +125,21 @@ Calling `timeout.unref()` creates an internal timer that will wake the Node.js
 event loop. Creating too many of these can adversely impact performance
 of the Node.js application.
 
+### `timeout[Symbol.toPrimitive]()`
+<!-- YAML
+added: REPLACEME
+-->
+
+* Returns: {integer} number that can be used to reference this `timeout`
+
+Coerce a `Timeout` to a primitive, a primitive will be generated that
+can be used to clear the `Timeout`.
+The generated number can only be used in the same thread where timeout
+was created. Therefore to use it cross [`worker_threads`][] it has
+to first be passed to a correct thread.
+This allows enhanced compatibility with browser's `setTimeout()`, and
+`setInterval()` implementations.
+
 ## Scheduling timers
 
 A timer in Node.js is an internal construct that calls a given function after
@@ -274,3 +289,4 @@ Cancels a `Timeout` object created by [`setTimeout()`][].
 [`setInterval()`]: timers.html#timers_setinterval_callback_delay_args
 [`setTimeout()`]: timers.html#timers_settimeout_callback_delay_args
 [`util.promisify()`]: util.html#util_util_promisify_original
+[`worker_threads`]: worker_threads.html
diff --git a/lib/internal/timers.js b/lib/internal/timers.js
@@ -103,6 +103,8 @@ const {
 const async_id_symbol = Symbol('asyncId');
 const trigger_async_id_symbol = Symbol('triggerId');
 
+const kHasPrimitive = Symbol('kHasPrimitive');
+
 const {
   ERR_INVALID_CALLBACK,
   ERR_OUT_OF_RANGE
@@ -182,6 +184,7 @@ function Timeout(callback, after, args, isRepeat, isRefed) {
   if (isRefed)
     incRefCount();
   this[kRefed] = isRefed;
+  this[kHasPrimitive] = false;
 
   initAsyncResource(this, 'Timeout');
 }
@@ -595,6 +598,7 @@ module.exports = {
   trigger_async_id_symbol,
   Timeout,
   kRefed,
+  kHasPrimitive,
   initAsyncResource,
   setUnrefTimeout,
   getTimerDuration,
diff --git a/lib/timers.js b/lib/timers.js
@@ -22,8 +22,10 @@
 'use strict';
 
 const {
+  ObjectCreate,
   MathTrunc,
   Promise,
+  SymbolToPrimitive
 } = primordials;
 
 const {
@@ -40,6 +42,7 @@ const {
     kRefCount
   },
   kRefed,
+  kHasPrimitive,
   initAsyncResource,
   getTimerDuration,
   timerListMap,
@@ -62,13 +65,21 @@ const {
   emitDestroy
 } = require('internal/async_hooks');
 
+// This stores all the known timer async ids to allow users to clearTimeout and
+// clearInterval using those ids, to match the spec and the rest of the web
+// platform.
+const knownTimersById = ObjectCreate(null);
+
 // Remove a timer. Cancels the timeout and resets the relevant timer properties.
 function unenroll(item) {
   if (item._destroyed)
     return;
 
   item._destroyed = true;
 
+  if (item[kHasPrimitive])
+    delete knownTimersById[item[async_id_symbol]];
+
   // Fewer checks may be possible, but these cover everything.
   if (destroyHooksExist() && item[async_id_symbol] !== undefined)
     emitDestroy(item[async_id_symbol]);
@@ -159,6 +170,14 @@ function clearTimeout(timer) {
   if (timer && timer._onTimeout) {
     timer._onTimeout = null;
     unenroll(timer);
+    return;
+  }
+  if (typeof timer === 'number' || typeof timer === 'string') {
+    const timerInstance = knownTimersById[timer];
+    if (timerInstance !== undefined) {
+      timerInstance._onTimeout = null;
+      unenroll(timerInstance);
+    }
   }
 }
 
@@ -204,6 +223,15 @@ Timeout.prototype.close = function() {
   return this;
 };
 
+Timeout.prototype[SymbolToPrimitive] = function() {
+  const id = this[async_id_symbol];
+  if (!this[kHasPrimitive]) {
+    this[kHasPrimitive] = true;
+    knownTimersById[id] = this;
+  }
+  return id;
+};
+
 const Immediate = class Immediate {
   constructor(callback, args) {
     this._idleNext = null;
diff --git a/test/parallel/test-timers-to-primitive.js b/test/parallel/test-timers-to-primitive.js
@@ -0,0 +1,29 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+[
+  setTimeout(common.mustNotCall(), 1),
+  setInterval(common.mustNotCall(), 1),
+].forEach((timeout) => {
+  assert.strictEqual(Number.isNaN(+timeout), false);
+  assert.strictEqual(+timeout, timeout[Symbol.toPrimitive]());
+  assert.strictEqual(`${timeout}`, timeout[Symbol.toPrimitive]().toString());
+  assert.deepStrictEqual(Object.keys({ [timeout]: timeout }), [`${timeout}`]);
+  clearTimeout(+timeout);
+});
+
+{
+  // Check that clearTimeout works with number id.
+  const timeout = setTimeout(common.mustNotCall(), 1);
+  const id = +timeout;
+  clearTimeout(id);
+}
+
+{
+  // Check that clearTimeout works with string id.
+  const timeout = setTimeout(common.mustNotCall(), 1);
+  const id = `${timeout}`;
+  clearTimeout(id);
+}
