node icon indicating copy to clipboard operation
node copied to clipboard

lib: mark process.nextTick legacy

Open marco-ippolito opened this issue 4 months ago • 13 comments

I propose to ~~documentation deprecate~~ mark as legacy process.nextTick, for the various inconsistencies and unpredictable behaviour. Some discussion happened here and here. ~~This won't probably transition to eol deprecation, but should limit the usage.~~

Refs: https://github.com/nodejs/node/issues/51156

@ronag @mcollina

marco-ippolito avatar Dec 24 '23 15:12 marco-ippolito

This should be using the doc: subsystem, as it's doc-only.

This will probably transition to eol deprecation, but should limit the usage.

Did you mean "won't"?

aduh95 avatar Dec 24 '23 15:12 aduh95

This should be using the doc: subsystem, as it's doc-only.

This will probably transition to eol deprecation, but should limit the usage.

Did you mean "won't"?

Yes it was a typo, it seems unrealistic to fully remove process.nextTick.

Fixed to the correct subsytem.

marco-ippolito avatar Dec 24 '23 16:12 marco-ippolito

I'm -1. Let's mark it legacy instead.

I don't think legacy will ever make a difference from the enduser perspective. What's wrong with document-only deprecating it?

anonrig avatar Dec 24 '23 16:12 anonrig

I think due to the inconsistent behavior it's better to deprecate it, if legacy even if unmaintaned should still somehow reliable, which is not. I think Legacy means, the API works but there are better and newer APIs to do the same thing, which I dont think is the case here.

marco-ippolito avatar Dec 24 '23 16:12 marco-ippolito

I don't think legacy will ever make a difference from the enduser perspective. What's wrong with document-only deprecating it?

@types/node will mark as deprecated, creating a maximum amount of noise for library maintainers and a lot of unneeded churn. Moreover, there is currently no way to achieve the same result right now.

I think due to the inconsistent behavior it's better to deprecate it, if legacy even if unmaintaned should still somehow reliable, which is not.

This is incorrenct. The function is reliable in what it does. The problem stems for the usage in combination with promises and EventEmitter, as showed in https://github.com/nodejs/node/issues/51156. When using it as originally intended, it's perfect.

Ultimately it's used everywhere in our codebase. We should not mark nextTick as doc-deprecated unless we have migrated ourselves off it.

mcollina avatar Dec 24 '23 17:12 mcollina

Event marking it legacy is likely a stretch right now, unless we have a dropped it from http, fs, streams and similar.

mcollina avatar Dec 24 '23 17:12 mcollina

#51267

ronag avatar Dec 24 '23 17:12 ronag

Ultimately it's used everywhere in our codebase. We should not mark nextTick as doc-deprecated unless we have migrated ourselves off it.

@ronag Isnt what this PR https://github.com/nodejs/node/pull/51279 goal is?

marco-ippolito avatar Dec 24 '23 17:12 marco-ippolito

Ultimately it's used everywhere in our codebase. We should not mark nextTick as doc-deprecated unless we have migrated ourselves off it.

@ronag Isnt what this PR https://github.com/nodejs/node/pull/51279 goal is?

The goal with that PR is to use the "correct" for node core until nextTick is "fixed". At that point we can replace deferTick with nextTick.

ronag avatar Dec 24 '23 17:12 ronag

marked as legacy

marco-ippolito avatar Dec 24 '23 17:12 marco-ippolito

queueMicrotask() takes a single parameter so it is not a drop-in replacement.

lpinca avatar Dec 25 '23 06:12 lpinca

On second thought, if we mark this as legacy, we should also mark legacy everywhere this is used. I don't think we should.

I have not done any analysis, but I think nextTick is also faster than queueMicrotask due to the forced closure allocation of the latter (as @lpinca said).

Let's wait until those are resolved to mark this one way or another.

mcollina avatar Dec 26 '23 08:12 mcollina

I think we should explain the problems with nextTick in the docs

rluvaton avatar Feb 13 '24 08:02 rluvaton