Remove `Symbol.dispose` integration in `setTimeout` and `setInterval`

[lucacasonato]

As part of the [Symbol.dispose] integration, the Timeout return type of setTimeout and setInterval support disposing (https://github.com/nodejs/node/pull/48633). This was stabilized last week (https://github.com/nodejs/node/pull/58467).

I think this integration should be removed, as the web platform setTimeout / setInterval do not support Symbol.dispose and never can, because it returns a number.

This means that folks may start writing new code that is incompatible between browser and Node, just because they use the Symbol.dispose integration here. That would be unfortunate.

It would also cause one more incompatibility between server side platforms with web setTimeout, and Node setTimeout (e.g Cloudflare Workers) (cc @jasnell).

@mcollina said in person that this was made stable recently enough to still remove it.

[mcollina]

cc @nodejs/tsc @jasnell

[jasnell]

Unfortunately, I think we'd likely need to take it through a deprecation cycle at this point but I'm fine with it.

[KhafraDev]

I don't understand. You can already write new code that is incompatible between browsers and node.js by using the returned Timeout object. I see plenty of new code use .unref() or .refresh().

[jasnell]

I think the concern is more about creating more of a discrepancy and introducing an additional, more obscure error condition. If we take the "It's a node.js timeout so it doesn't matter" point of view then we likely are doing a disservice for cross-runtime interop/portability.

[LiviaMedeiros]

Would it be feasible on the ~~very~~ long term to have separate timers implementations? I.e. globalThis.setTimeout() that tries its best to follow the HTML spec, and node:timers.setTimeout()&node:timers/promises.setTimeout() that are not bound by interoperability constraints anymore.

[Ethan-Arrowood]

I think we should just create new Node.js timer APIs that do all the things we want (return objects, disposable) and then we aren't tied to browser spec. We should support setTimeout and setImmediate as standardized, and then have separate APIs (call them delay or sleep or whatever else) and then it's the users choice if they need interop code to use the browser ones, and if not use the node ones.

[jasnell]

Sadly, while I agree in concept, I suspect that deprecating our current setTimeout/setInterval return values would be quite difficult... On par with the deprecation of the Buffer constructor. I don't know if we'd be able to get there.

[LiviaMedeiros]

Any existing userland code that uses Node.js is using the node:timers implementation. How about this:

1. Deprecate accessing setTimeout/setInterval/ etc. as globals in favour of explicitly importing them from node:timers.
2. Keep the implementation independent from the HTML spec.
3. After EOL, reintroduce globalThis.setTimeout with behavior as close to the browser version as possible.

For users relying on Node.js's implementation, adding explicit import/require (which already should be a recommended practice) would be sufficient. They will retain full functionality with promises, disposers, refresh, etc. For users using globalThis.setTimeout in browser-compatible way, nothing would change and they may get even more compatibility (such as implicit eval...).

IMHO this is much better than being stuck forever with an API that is both incompatible and unable to evolve.

[KhafraDev]

I think we are steering away from the topic - if there are no plans of deprecating or changing the return type, then I don't really see the justification for removing Symbol.dispose from Timeout.

I don't think it should have been added and I wish timers returned a number, but if the reason for removing something is "this will benefit Deno/Cloudflare workers", I am not sure why we should support it. The same reason I supported passing async iterables to Response despite not being a fan of the change originally.

[jasnell]

At this point, with my TSC hat firmly in place, largely because the disposal symbols have been in place for multiple major release cycles now, and because although official support in v8 for using only just landed, TypeScript and polyfills have been in general use now for a while, I believe a deprecation cycle is mandatory for removing these.

Secondly, I agree they can/should be removed in order to best facilitate interoperability across the ecosystem.

[mhdawson]

For the TSC members (13) who were in the meeting the consensus was that it would need to go through a deprecation cycle before being removed (which people agreed is unfortunate).

Removing from the TSC agenda.