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

add .fork option #18

Merged
merged 3 commits into from
Feb 16, 2017
Merged

add .fork option #18

merged 3 commits into from
Feb 16, 2017

Conversation

juliangruber
Copy link
Contributor

No description provided.

@mafintosh mafintosh merged commit 0211437 into mafintosh:master Feb 16, 2017
@mafintosh
Copy link
Owner

2.5.0

@juliangruber juliangruber deleted the add/fork branch February 16, 2017 17:43
@01binary
Copy link

@juliangruber can you explain why the fork option tries to find a node module instead of an executable?

This seems like an API-breaking change:

  • Without fork option (default): command is a path to an executable file (.exe on Windows)
  • With fork option: command is a path to a JavaScript file that will be run with node as a module

We use respawn to launch using different node versions, so the addition of fork option now attempts to load node.exe or node64.exe as a node module. I get a JavaScript parse error that the .exe is not a JavaScript file.

i.e. we go

c:\program files\custom-node\node64.exe ourmodule.js

and it loads

c:\program files\custom-node\node64.exe as a node module instead of loading ourmodule.js through the specified node exe

Anything can be done about this, PR etc?

@01binary
Copy link

See #21

@juliangruber
Copy link
Contributor Author

That's how child_process.fork works, see https://nodejs.org/api/child_process.html#child_process_child_process_fork_modulepath_args_options. It needs the path to a node module instead of an executable.

If {fork:false}, this is using child_process.spawn just as before.

@01binary
Copy link

Thanks - just confusing because setting one flag in options changes the API almost entirely.

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.

3 participants