Skip to content

doc: fix worker example to receive message - #21486

Closed
thefourtheye wants to merge 1 commit into
nodejs:masterfrom
thefourtheye:fix-worker-threads-doc
Closed

doc: fix worker example to receive message#21486
thefourtheye wants to merge 1 commit into
nodejs:masterfrom
thefourtheye:fix-worker-threads-doc

Conversation

@thefourtheye

@thefourtheyethefourtheye commented Jun 23, 2018

Copy link
Copy Markdown
Contributor

require('worker_threads') is not an instance of EventEmitter. So
once method would not be in it. The correct way to receive the message
would be to attach a listener to the message event on the
parentPort.

Also, there is no built-in event called workerMessage. This patch
fixes it by referencing message event.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@thefourtheyethefourtheye added doc Issues and PRs related to the documentations. worker Issues and PRs related to Worker support. labels Jun 23, 2018
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-botnodejs-github-bot added the doc Issues and PRs related to the documentations. label Jun 23, 2018
Comment threaddoc/api/worker_threads.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think in this case the norm is to link worker_threads.parentPort and 'message' separately, to something like

received via [`worker_threads.parentPort`][]'s [`'message'`][`port.on('message')`] event.

@vssenko

Copy link
Copy Markdown

Wow, luckly i found this issue, helped a lot. But documentation is still invalid.

@TimothyGuTimothyGu mentioned this pull request Jul 9, 2018
2 tasks
@addaleax

Copy link
Copy Markdown
Member

@thefourtheye Can you rebase this?

@jasnell

Copy link
Copy Markdown
Member

ping @thefourtheye

@thefourtheye
thefourtheyeforce-pushed the fix-worker-threads-doc branch from f80beff to 9bc522dCompareJuly 17, 2018 04:46
`require('worker_threads')` is not an instance of `EventEmitter`. So
`on` method would not be in it. The correct way to receive the message
would be to attach a listener to the `message` event on the
`parentPort`.
@thefourtheye
thefourtheyeforce-pushed the fix-worker-threads-doc branch from 9bc522d to d1b0121CompareJuly 17, 2018 04:50
@thefourtheye

Copy link
Copy Markdown
ContributorAuthor

@addaleax@jasnell Sorry for the delay. Rebased the PR now.

@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 18, 2018
@addaleax

Copy link
Copy Markdown
Member

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jul 18, 2018
`require('worker_threads')` is not an instance of `EventEmitter`. So
`on` method would not be in it. The correct way to receive the message
would be to attach a listener to the `message` event on the
`parentPort`.
PR-URL: nodejs#21486
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR

Copy link
Copy Markdown
Member

Landed in 42be4c3 🎉

@thefourtheye
thefourtheye deleted the fix-worker-threads-doc branch July 18, 2018 17:14
@targostargos removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 19, 2018
targos pushed a commit that referenced this pull request Jul 19, 2018
`require('worker_threads')` is not an instance of `EventEmitter`. So
`on` method would not be in it. The correct way to receive the message
would be to attach a listener to the `message` event on the
`parentPort`.
PR-URL: #21486
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@targostargos mentioned this pull request Jul 31, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.workerIssues and PRs related to Worker support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@thefourtheye@nodejs-github-bot@vssenko@addaleax@jasnell@BridgeAR@TimothyGu@cjihrig@vsemozhetbyt@targos