-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
feat: allow movable workerData
#27
Conversation
* Handle transferable `data` or transferable properties of `data` by unwrapping | ||
* them and adding their transferable objects to the given `transferList` array. | ||
*/ | ||
function fillTransferList(data: any, transferList: TransferListItem[]): any { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the star of the show. The rest is mainly TypeScript improvements.
@@ -254,23 +245,8 @@ class TaskInfo extends AsyncResource implements Task { | |||
) { | |||
super('Tinypool.Task', { requireManualDestroy: true, triggerAsyncId }) | |||
this.callback = callback | |||
this.task = task | |||
this.task = fillTransferList(task, transferList) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR also adds nested movable support to the pool.run
method. ☝️
Note that only 1 level of nesting is checked. In other words, doing this 👇 is not supported:
pool.run({ a: { b: Tinypool.move(b) } })
@@ -3,6 +3,7 @@ import { | |||
MessageChannel, | |||
MessagePort, | |||
receiveMessageOnPort, | |||
TransferListItem, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out worker_threads
exports a TransferListItem
type for us, so we don't have to extract it from MessagePort.postMessage
anymore :)
@aleclarson Thank you, Alec, This looks good overall, though, we need to add tests for it and a short description in README! |
ea8fa68
to
b68456b
Compare
@aleclarson Any updates on this? |
Closes #26