std/node: Node compatible setTimeout, setInterval

[kt3k]

setTimeout in Node.js returns Timeout object

ref(Node.js):

• https://github.com/nodejs/node/blob/8ee4e672eca6826e7b6f2c1ad36ecbe6e570d412/lib/timers.js#L140
• https://github.com/nodejs/node/blob/ee9e2a2eb6143fa7b4b1454f7aed009a8703d4d7/lib/internal/timers.js#L167

We have Timeout class implemented in std/node, but doesn't seem implementing setTimeout correctly

• Timeout https://github.com/denoland/deno_std/blob/ce0ac6ec4b489ec3eab7bbf4991efb8a0c9fe827/node/_timers.ts#L31
• setTimeout https://github.com/denoland/deno_std/blob/ce0ac6ec4b489ec3eab7bbf4991efb8a0c9fe827/node/timers.ts

[bartlomieju]

I'm not sure how we can do that, it's the same problem as in #1455.

I'm very much against overriding globals depending if we're in compat mode or Node, ie. we should stick to Deno's globals anyway. I discussed it with @dsherret, but there are no ideas how to reconcile the two, the globals are just incompatible.

@bnoordhuis any ideas?

[kt3k]

But this difference actually prevents running mocha's integration test in compat mode as they use setInterval().unref() in their testing utility. ref: https://github.com/mochajs/mocha/blob/3b4cc05f3fa0502d5d714a8f62ac8785267aa695/lib/nodejs/parallel-buffered-runner.js#L307-L310

You can reproduce the above issue with the following commands.

git clone https://github.com/mochajs/mocha.git
cd mocha
npm install
# run integration test
deno run --unstable --compat -A --location https://deno.land bin/mocha --parallel --timeout 10000 --slow 3750 "test/integration/**/*.spec.js"

How about overriding these in compat mode, but discouraging the use of these non-standard features somehow, for example, by warning in the linter

[lucacasonato]

I'd say it is probably safe to override them in compat mode, if Symbo.toPrimitive is implemented on the timers object. Very ugly though (and very painful). See responses to https://twitter.com/lcasdev/status/1462766579196862470

[Jack-Works]

I'd say it is probably safe to override them in compat mode, if Symbo.toPrimitive is implemented on the timers object. Very ugly though (and very painful). See responses to https://twitter.com/lcasdev/status/1462766579196862470

Node also has Symbol.toPrimitive on its timer class.

[kt3k]

The issue denoland/deno#12745 might be related if we want to implement setInterval().unref()

[bnoordhuis]

Channeling @lucacasonato: idea is to monkey-patch setTimeout/setInterval globals and return objects instead of numbers in --compat mode.

(My recollection was to return objects always but I might be misremembering and I don't have a strong preference anyway.)