Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate process.nextTick in favor of queueMicrotask #36870

Closed
martinheidegger opened this issue Jan 10, 2021 · 9 comments
Closed

Deprecate process.nextTick in favor of queueMicrotask #36870

martinheidegger opened this issue Jan 10, 2021 · 9 comments
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.

Comments

@martinheidegger
Copy link

Trying to work out some things with readable-stream recently and I noticed that queueMicrotask seems to be sharing the same operational paradigm as process.nextTick (as in: same time & style of execution). It is native to the v8 and available in browsers, has some async hooks available so I was wondering if - long term and in the spirit of cross-platform compatibility - to shift, little-by-little, from process.nextTick to queueMicrotask? 😅

The request to deprecate may be a bit much, as process.nextTick is probably used everywhere and I was wondering if a soft-deprecation may be a good idea: i.e. mention that queueMicrotask is the preferred way to go and that it only exists for legacy reasons? And maybe not use it for new API's?

@PoojaDurgad PoojaDurgad added the stream Issues and PRs related to the stream subsystem. label Jan 11, 2021
@aduh95 aduh95 added lib / src Issues and PRs related to general changes in the lib or src directory. and removed stream Issues and PRs related to the stream subsystem. labels Jan 11, 2021
@jasnell
Copy link
Member

jasnell commented Feb 22, 2021

As you suggest, process.nextTick() is used far too extensively to be deprecated. The other issue is that the two methods (nextTick() and queueMicrotask()) serve two entirely different goals at a purely technical level. They certainly appear similar and are probably interchangeable in most userland cases, there are situations where switching to queueMicrotask() could be a performance loss and a breaking change in timing expectations (albeit unlikely). As an alternative to deprecation, we likely should document the differences between the two and give specific recommendations on when they should be used -- with deference given to queueMicrotask() for most userland use cases. /cc @mcollina

jasnell added a commit to jasnell/node that referenced this issue Feb 22, 2021
We likely cannot ever deprecate process.nextTick, but we can start
steering people towards queueMicrotask for most cases.

Signed-off-by: James M Snell <[email protected]>
Fixes: nodejs#36870
@jasnell
Copy link
Member

jasnell commented Feb 22, 2021

PR: #37484

@mcollina
Copy link
Member

The assessment @jasnell did is correct. I don't think we'll be able to remove process.nextTick(), it's everywhere.

jasnell added a commit to jasnell/node that referenced this issue Feb 23, 2021
We likely cannot ever deprecate process.nextTick, but we can start
steering people towards queueMicrotask for most cases.

Signed-off-by: James M Snell <[email protected]>
Fixes: nodejs#36870
@ronag
Copy link
Member

ronag commented Feb 24, 2021

Since it seems slower than nextTick. I'm not sure everyone agrees nextTick should be deprecated. We should though be more explicit about the edge cases with nextTick.

@vweevers
Copy link
Contributor

Does the "write idiomatic JavaScript, we will optimize" rule apply here? queueMicrotask being the idiomatic API.

@ronag

This comment has been minimized.

@jasnell
Copy link
Member

jasnell commented Feb 24, 2021

Likely not without it being a significant breaking change. The fact that queueMicrotask() tasks are scheduled in sequence with Promise then/catch/finally handlers would make it a pretty significant change in the timing contract.

Consider the following examples:

const p = Promise.resolve();

p.then(() => {
	process.nextTick(() => console.log('a'));
})

p.then(() => {
	queueMicrotask(() => console.log('b'));
})

p.finally(() => {
	  console.log('c');
});

The order in which the items are printed here is: c, b, a.

If you change the process.nextTick() into a queueMicrotask...

const p = Promise.resolve();

p.then(() => {
	queueMicrotask(() => console.log('a'));
//	process.nextTick(() => console.log('a'));
})

p.then(() => {
	queueMicrotask(() => console.log('b'));
})

p.finally(() => {
	  console.log('c');
});

The order becomes c, a, b

While the change is extremely subtle on the surface, it can cause significant problems in practice, particularly with code that's been written to expect a particular timing.

That's not to say we shouldn't make that kind of change! It just means we have to be careful and understand that it could definitely break things.

@martinheidegger
Copy link
Author

@jasnell The PR is pretty nice! Thank you for picking this up 😻

It does seem to me like @vweevers point is valid as in: use of queueMicrotask should over time should be same fast / faster. Very curious why it isnt.

In any case: I agree that a change of a current API does not make sense. But do new API's need to on the "nextTick" timing?

@jasnell
Copy link
Member

jasnell commented Feb 24, 2021

It's really not a matter of the performance difference between queueMicrotask() and process.nextTick()... the two should have nearly identical performance profiles. The issue comes down specifically to the timing and order of execution. The sentiment in @vweevers comment is correct, however, queueMicrotask() has become the more idiomatic multi-platform approach and should be favored by the overwhelming majority of user code cases, with process.nextTick() falling solely in the realm of Node.js core and library authors.

targos pushed a commit that referenced this issue Feb 28, 2021
We likely cannot ever deprecate process.nextTick, but we can start
steering people towards queueMicrotask for most cases.

Signed-off-by: James M Snell <[email protected]>
Fixes: #36870

PR-URL: #37484
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
targos pushed a commit that referenced this issue May 1, 2021
We likely cannot ever deprecate process.nextTick, but we can start
steering people towards queueMicrotask for most cases.

Signed-off-by: James M Snell <[email protected]>
Fixes: #36870

PR-URL: #37484
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants