★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/21486Nouvelle recherche | Portail wanayoo
Skip to content

/node

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

doc: fix worker example to receive message #21486

Closed
wants to merge 1 commit into from

Conversation

Reviewers

@jasnell jasnell

@addaleax addaleax

@TimothyGu TimothyGu

@cjihrig cjihrig

@BridgeAR BridgeAR

@vsemozhetbyt vsemozhetbyt

Assignees
No one assigned
Labels
doc worker
Projects
None yet
Milestone
No milestone
10 participants
@thefourtheye @nodejs-github-bot @vssenko @addaleax @jasnell @BridgeAR @TimothyGu @cjihrig @vsemozhetbyt @targos
@thefourtheye
Copy link
Contributor

thefourtheye commented Jun 23, 2018 •
edited

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

@thefourtheye thefourtheye added doc worker labels Jun 23, 2018

@thefourtheye thefourtheye requested a review from addaleax Jun 23, 2018

@nodejs-github-bot

@nodejs-github-bot nodejs-github-bot added the doc label Jun 23, 2018

@TimothyGu

TimothyGu approved these changes Jun 23, 2018
View changes

doc/api/worker_threads.md Outdated
@@ -375,7 +376,7 @@ added: v10.5.0
* `transferList` {Object[]}

Send a message to the worker that will be received via
[`require('worker_threads').on('workerMessage')`][].
[`require('worker_threads').parentPort.on('message')`][].

This comment has been minimized.

Sign in to view
Copy link
@TimothyGu

TimothyGu Jun 23, 2018

Member

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.
@vsemozhetbyt

vsemozhetbyt approved these changes Jun 23, 2018
View changes

@jasnell

jasnell approved these changes Jun 24, 2018
View changes

@vssenko

This comment has been minimized.

Sign in to view
Copy link

vssenko commented Jun 29, 2018

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

@TimothyGu TimothyGu referenced this pull request Jul 9, 2018

Closed

tools: add no-duplicate-requires rule #21712

2 of 2 tasks complete
@addaleax

This comment has been minimized.

Sign in to view
Copy link
Member

addaleax commented Jul 9, 2018

@thefourtheye Can you rebase this?

@vsemozhetbyt vsemozhetbyt referenced this pull request Jul 10, 2018

Closed

some code example outdated in the doc for worker_threads #21714

@jasnell

This comment has been minimized.

Sign in to view
Copy link
Member

jasnell commented Jul 12, 2018

ping @thefourtheye

@thefourtheye thefourtheye force-pushed the thefourtheye:fix-worker-threads-doc branch from f80beff to 9bc522d Jul 17, 2018

@thefourtheye
doc: fix worker example to receive message 
`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`.
Unverified
The key that signed this is expired.
GPG key ID: C71F2437E9591758 Learn about signing commits
Loading status checks…
d1b0121

@thefourtheye thefourtheye force-pushed the thefourtheye:fix-worker-threads-doc branch from 9bc522d to d1b0121 Jul 17, 2018

@thefourtheye

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

thefourtheye commented Jul 17, 2018

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

cjihrig approved these changes Jul 17, 2018
View changes

@addaleax

addaleax approved these changes Jul 18, 2018
View changes

@addaleax addaleax added the author ready label Jul 18, 2018

@addaleax

This comment has been minimized.

Sign in to view
Copy link
Member

addaleax commented Jul 18, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request-lite/914/
@BridgeAR

BridgeAR approved these changes Jul 18, 2018
View changes

BridgeAR added a commit to BridgeAR/node that referenced this pull request Jul 18, 2018

@thefourtheye @BridgeAR
doc: fix worker example to receive message 
`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>
Verified
This commit was signed with a verified signature.
@BridgeAR BridgeAR Ruben Bridgewater
GPG key ID: F07496B3EB3C1762 Learn about signing commits
42be4c3
@BridgeAR

This comment has been minimized.

Sign in to view
Copy link
Member

BridgeAR commented Jul 18, 2018

Landed in 42be4c3 🎉

@BridgeAR BridgeAR closed this Jul 18, 2018

@thefourtheye thefourtheye deleted the thefourtheye:fix-worker-threads-doc branch Jul 18, 2018

@targos targos removed the author ready label Jul 19, 2018

targos added a commit that referenced this pull request Jul 19, 2018

@thefourtheye @targos
doc: fix worker example to receive message 
`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>
Verified
This commit was signed with a verified signature.
@targos targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
b510cdc

@targos targos referenced this pull request Jul 31, 2018

Merged

v10.8.0 proposal #22040

@beevelop beevelop referenced this pull request Aug 1, 2018

Closed

2018-08-01 Version 10.8.0 (Current) @targos #212

This was referenced Aug 1, 2018

Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.