Skip to content

process: add process.runNextTicks() method #40088

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 13 commits into from
Closed

process: add process.runNextTicks() method #40088

wants to merge 13 commits into from

Conversation

@Richienb
Copy link
Contributor

Finishes #29671

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Sep 12, 2021
@Richienb Richienb changed the title Add process.runNextTicks() method process: add process.runNextTicks() method Sep 12, 2021
mscdex
mscdex reviewed Sep 12, 2021
View reviewed changes
mscdex
mscdex reviewed Sep 12, 2021
View reviewed changes
Richienb and others added 3 commits September 12, 2021 17:45
@Richienb @mscdex
Update doc/api/process.md
ca914cc 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
@Richienb @mscdex
Update doc/api/process.md
6a37928 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
@Richienb
Update process.md
1070316
mscdex
mscdex reviewed Sep 12, 2021
View reviewed changes
Mesteery
Mesteery reviewed Sep 12, 2021
View reviewed changes
@Richienb Richienb requested review from Mesteery and mscdex September 12, 2021 09:49
addaleax
addaleax requested changes Sep 12, 2021
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would need a very strong reason if we wanted to make this public API, given that it is intentional that this is neither part of the language nor other runtime environments like browsers.

@Richienb
Copy link
Contributor Author

If #40054 is not possible, this is the next best thing to achieve the same result.

@addaleax
Copy link
Member

@Richienb I think this PR would go waaaaay further than what is being suggested in #40054. This would enable running code that behaves as asynchronous in every JS environment as synchronous code; that’s a big footgun.

Also, I just commented on #40054, it is possible to do this with existing APIs.

@jasnell
Copy link
Member

jasnell commented Sep 12, 2021

I'm definitely in agreement with @addaleax ... really not convinced this is something we should do.

ronag
ronag requested changes Sep 15, 2021
View reviewed changes
Copy link
Member

@ronag ronag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems dangerous and easily misused. Do we have a motivating example for why this would be needed?

@Richienb
Copy link
Contributor Author

Since #40054 has been proven possible, this PR is no longer needed.

@Richienb Richienb closed this Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax requested changes

@ronag ronag ronag requested changes

@mscdex mscdex Awaiting requested review from mscdex

@Mesteery Mesteery Awaiting requested review from Mesteery

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Richienb @addaleax @jasnell @mscdex @ronag @Mesteery @nodejs-github-bot