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

Skip redirecting stdout for Worker threads #208

Merged
merged 1 commit into from
Feb 23, 2019

Conversation

addaleax
Copy link
Contributor

This breaks running 0x with Workers, because the internal stdio
implementation for Workers currently relies on process.stdout
being the originally provided object.

Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

This is the only thing that's needed! Wow!

@@ -1,6 +1,11 @@
'use strict'
const net = require('net')

try {
// Skip redirecting stdout in Worker threads.
if (!require('worker_threads').isMainThread) return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lint is failing. Would you mind adding an exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mcollina Ugh, just seeing that … since this is a parsing error for the linter, not a linter error, I’m not sure there’s a way to add an exception…?

I’ve worked around this the awkward way, wrapping the rest of the file in if (!isWorker). I hope that’s okay?

This breaks running 0x with Workers, because the internal stdio
implementation for Workers currently relies on `process.stdout`
being the originally provided object.
Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina
Copy link
Collaborator

Thanks!

@mcollina mcollina merged commit f919655 into davidmarkclements:master Feb 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants