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

process: refactor global.queueMicrotask() #26523

Closed
wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

  • Lazy load async_hooks in the implementation
  • Rename process/next_tick.js to process/task_queues.js
    and move the implementation of global.queueMicrotask()
    there since these methods are conceptually related to
    each other.
  • Move the bindings used by global.queueMicrotask() into
    node_task_queue.cc instead of the generic node_util.cc
  • Use defineOperation to define global.queueMicrotask()
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

- Lazy load `async_hooks` in the implementation
- Rename `process/next_tick.js` to `process/task_queues.js`
  and move the implementation of `global.queueMicrotask()`
  there since these methods are conceptually related to
  each other.
- Move the bindings used by `global.queueMicrotask()` into
  `node_task_queue.cc` instead of the generic `node_util.cc`
- Use `defineOperation` to define `global.queueMicrotask()`
@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Mar 8, 2019
@joyeecheung joyeecheung requested a review from devsnek March 8, 2019 15:44
@joyeecheung
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/21337/

This is sort-of an alternative to #26520 but it's certainly not fast-trackable.

@joyeecheung joyeecheung added the process Issues and PRs related to the process subsystem. label Mar 8, 2019
@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 8, 2019
@danbev
Copy link
Contributor

danbev commented Mar 11, 2019

Landed in 8d669bb.

@danbev danbev closed this Mar 11, 2019
danbev pushed a commit that referenced this pull request Mar 11, 2019
- Lazy load `async_hooks` in the implementation
- Rename `process/next_tick.js` to `process/task_queues.js`
  and move the implementation of `global.queueMicrotask()`
  there since these methods are conceptually related to
  each other.
- Move the bindings used by `global.queueMicrotask()` into
  `node_task_queue.cc` instead of the generic `node_util.cc`
- Use `defineOperation` to define `global.queueMicrotask()`

PR-URL: #26523
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR
Copy link
Member

This does not land cleanly on v11. It seems to rely on other commits that should be backported first. It might land cleanly after landing the other backport requested PRs.

targos pushed a commit that referenced this pull request Mar 27, 2019
- Lazy load `async_hooks` in the implementation
- Rename `process/next_tick.js` to `process/task_queues.js`
  and move the implementation of `global.queueMicrotask()`
  there since these methods are conceptually related to
  each other.
- Move the bindings used by `global.queueMicrotask()` into
  `node_task_queue.cc` instead of the generic `node_util.cc`
- Use `defineOperation` to define `global.queueMicrotask()`

PR-URL: #26523
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. lib / src Issues and PRs related to general changes in the lib or src directory. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants